mbox series

[0/6] Support second Image Signal Processor on rk3399

Message ID 20210202145632.1263136-1-heiko@sntech.de (mailing list archive)
Headers show
Series Support second Image Signal Processor on rk3399 | expand

Message

Heiko Stübner Feb. 2, 2021, 2:56 p.m. UTC
The rk3399 has two ISPs and right now only the first one is usable.
The second ISP is connected to the TXRX dphy on the soc.

The phy of ISP1 is only accessible through the DSI controller's
io-memory, so this series adds support for simply using the dsi
controller is a phy if needed.

That solution is needed at least on rk3399 and rk3288 but no-one
has looked at camera support on rk3288 at all, so right now
only implement the rk3399 specifics.


Heiko Stuebner (6):
  drm/rockchip: dsi: add own additional pclk handling
  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
  arm64: dts: rockchip: add isp1 node on rk3399

 .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
 drivers/gpu/drm/rockchip/Kconfig              |   2 +
 .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
 4 files changed, 384 insertions(+)

Comments

Sebastian Fricke Feb. 3, 2021, 6:14 p.m. UTC | #1
Hey Heiko,

I have tested your patch set on my nanoPC-T4, here is a complete log
with:
- relevant kernel log entries
- system information
- media ctl output
- sysfs entry information

https://paste.debian.net/1183874/

Additionally, to your patchset I have applied the following patches:
https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup

And just to not cause confusion the `media_dev` entries come from this
unmerged series:
https://patchwork.kernel.org/project/linux-media/list/?series=426269

I have actually been able to stream with both of my cameras at the same
time using the libcamera cam command.
I would like to thank you a lot for making this possible.

If you like to you can add:
Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>

On 02.02.2021 15:56, Heiko Stuebner wrote:
>The rk3399 has two ISPs and right now only the first one is usable.
>The second ISP is connected to the TXRX dphy on the soc.
>
>The phy of ISP1 is only accessible through the DSI controller's
>io-memory, so this series adds support for simply using the dsi
>controller is a phy if needed.
>
>That solution is needed at least on rk3399 and rk3288 but no-one
>has looked at camera support on rk3288 at all, so right now
>only implement the rk3399 specifics.
>
>
>Heiko Stuebner (6):
>  drm/rockchip: dsi: add own additional pclk handling
>  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>  arm64: dts: rockchip: add isp1 node on rk3399
>
> .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> drivers/gpu/drm/rockchip/Kconfig              |   2 +
> .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> 4 files changed, 384 insertions(+)
>
>-- 
>2.29.2
>
Heiko Stübner Feb. 3, 2021, 7:54 p.m. UTC | #2
Hi Sebastian,

Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> Hey Heiko,
> 
> I have tested your patch set on my nanoPC-T4, here is a complete log
> with:
> - relevant kernel log entries
> - system information
> - media ctl output
> - sysfs entry information
> 
> https://paste.debian.net/1183874/
> 
> Additionally, to your patchset I have applied the following patches:
> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> 
> And just to not cause confusion the `media_dev` entries come from this
> unmerged series:
> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> 
> I have actually been able to stream with both of my cameras at the same
> time using the libcamera cam command.
> I would like to thank you a lot for making this possible.

Thanks for testing a dual camera setup. On my board I could only test
the second ISP. And really glad it works for you tool :-) .

Out of curiosity, do you also see that green tint in the images the cameras
produce?

Thanks
Heiko


> If you like to you can add:
> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> 
> On 02.02.2021 15:56, Heiko Stuebner wrote:
> >The rk3399 has two ISPs and right now only the first one is usable.
> >The second ISP is connected to the TXRX dphy on the soc.
> >
> >The phy of ISP1 is only accessible through the DSI controller's
> >io-memory, so this series adds support for simply using the dsi
> >controller is a phy if needed.
> >
> >That solution is needed at least on rk3399 and rk3288 but no-one
> >has looked at camera support on rk3288 at all, so right now
> >only implement the rk3399 specifics.
> >
> >
> >Heiko Stuebner (6):
> >  drm/rockchip: dsi: add own additional pclk handling
> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> >  arm64: dts: rockchip: add isp1 node on rk3399
> >
> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> > 4 files changed, 384 insertions(+)
> >
>
Sebastian Fricke Feb. 5, 2021, 6:43 a.m. UTC | #3
Hey Heiko,

On 03.02.2021 20:54, Heiko Stübner wrote:
>Hi Sebastian,
>
>Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
>> Hey Heiko,
>>
>> I have tested your patch set on my nanoPC-T4, here is a complete log
>> with:
>> - relevant kernel log entries
>> - system information
>> - media ctl output
>> - sysfs entry information
>>
>> https://paste.debian.net/1183874/
>>
>> Additionally, to your patchset I have applied the following patches:
>> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
>>
>> And just to not cause confusion the `media_dev` entries come from this
>> unmerged series:
>> https://patchwork.kernel.org/project/linux-media/list/?series=426269
>>
>> I have actually been able to stream with both of my cameras at the same
>> time using the libcamera cam command.
>> I would like to thank you a lot for making this possible.
>
>Thanks for testing a dual camera setup. On my board I could only test
>the second ISP. And really glad it works for you tool :-) .
>
>Out of curiosity, do you also see that green tint in the images the cameras
>produce?

Yes, I do. Actually, I currently have two forms of a green tint, on my
OV13850 everything is quite dark and greenish, which is caused by the
missing 3A algorithms. On my OV4689, I have big patches of the image
with bright green color and flickering, I investigated if this is
connected to the 2nd ISP instance, but that doesn't seem to be the case
as I have the same results when I switch the CSI ports of the cameras.

I have found another issue, while testing I discovered following
issue:
When I start the system with an HDMI monitor connected, then the camera
on the 2nd port doesn't work. This is probably because the RX/TX is
reserved as a TX.
But it made me wonder because if the system has an RX, a TX, and
an RX/TX, why isn't the pure TX used by the monitor and the
cameras take RX and RX/TX?
Or do you think that this is maybe a malfunction of this patch?

>
>Thanks
>Heiko

Greetings,
Sebastian

>
>
>> If you like to you can add:
>> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
>>
>> On 02.02.2021 15:56, Heiko Stuebner wrote:
>> >The rk3399 has two ISPs and right now only the first one is usable.
>> >The second ISP is connected to the TXRX dphy on the soc.
>> >
>> >The phy of ISP1 is only accessible through the DSI controller's
>> >io-memory, so this series adds support for simply using the dsi
>> >controller is a phy if needed.
>> >
>> >That solution is needed at least on rk3399 and rk3288 but no-one
>> >has looked at camera support on rk3288 at all, so right now
>> >only implement the rk3399 specifics.
>> >
>> >
>> >Heiko Stuebner (6):
>> >  drm/rockchip: dsi: add own additional pclk handling
>> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>> >  arm64: dts: rockchip: add isp1 node on rk3399
>> >
>> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
>> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
>> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
>> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
>> > 4 files changed, 384 insertions(+)
>> >
>>
>
>
>
>
Heiko Stübner Feb. 5, 2021, 8:15 a.m. UTC | #4
Hi Sebastian,

Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> On 03.02.2021 20:54, Heiko Stübner wrote:
> >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> >> I have tested your patch set on my nanoPC-T4, here is a complete log
> >> with:
> >> - relevant kernel log entries
> >> - system information
> >> - media ctl output
> >> - sysfs entry information
> >>
> >> https://paste.debian.net/1183874/
> >>
> >> Additionally, to your patchset I have applied the following patches:
> >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> >>
> >> And just to not cause confusion the `media_dev` entries come from this
> >> unmerged series:
> >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> >>
> >> I have actually been able to stream with both of my cameras at the same
> >> time using the libcamera cam command.
> >> I would like to thank you a lot for making this possible.
> >
> >Thanks for testing a dual camera setup. On my board I could only test
> >the second ISP. And really glad it works for you tool :-) .
> >
> >Out of curiosity, do you also see that green tint in the images the cameras
> >produce?
> 
> Yes, I do. Actually, I currently have two forms of a green tint, on my
> OV13850 everything is quite dark and greenish, which is caused by the
> missing 3A algorithms. On my OV4689, I have big patches of the image
> with bright green color and flickering, I investigated if this is
> connected to the 2nd ISP instance, but that doesn't seem to be the case
> as I have the same results when I switch the CSI ports of the cameras.
> 
> I have found another issue, while testing I discovered following
> issue:
> When I start the system with an HDMI monitor connected, then the camera
> on the 2nd port doesn't work. This is probably because the RX/TX is
> reserved as a TX.
> But it made me wonder because if the system has an RX, a TX, and
> an RX/TX, why isn't the pure TX used by the monitor and the
> cameras take RX and RX/TX?
> Or do you think that this is maybe a malfunction of this patch?

I don't think it is an issue with this specific series, but still puzzling.

I.e. the DPHYs are actually only relevant to the DSI controllers,
with TX0 being connected to DSI0 and TX1RX1 being connected
to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.

Out of curiosity what happens, when you boot without hdmi connected
turn on the cameras, connect the hdmi after this, try the cameras again?


Heiko

> 
> >
> >Thanks
> >Heiko
> 
> Greetings,
> Sebastian
> 
> >
> >
> >> If you like to you can add:
> >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> >>
> >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> >> >The rk3399 has two ISPs and right now only the first one is usable.
> >> >The second ISP is connected to the TXRX dphy on the soc.
> >> >
> >> >The phy of ISP1 is only accessible through the DSI controller's
> >> >io-memory, so this series adds support for simply using the dsi
> >> >controller is a phy if needed.
> >> >
> >> >That solution is needed at least on rk3399 and rk3288 but no-one
> >> >has looked at camera support on rk3288 at all, so right now
> >> >only implement the rk3399 specifics.
> >> >
> >> >
> >> >Heiko Stuebner (6):
> >> >  drm/rockchip: dsi: add own additional pclk handling
> >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> >> >  arm64: dts: rockchip: add isp1 node on rk3399
> >> >
> >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> >> > 4 files changed, 384 insertions(+)
> >> >
> >>
> >
> >
> >
> >
>
Heiko Stübner Feb. 5, 2021, 2:55 p.m. UTC | #5
Hi Sebastian,

I did some tests myself today as well and can confirm your
hdmi related finding - at least when plugged in on boot.

I tried some combinations of camera vs. hdmi and it seems
really only when hdmi is plugged in on boot

(1)
- boot
- camera
--> works

(2)
- boot
- camera
- hdmi plugged in
- hdmi works
- camera
--> works

(3)
- hdmi plugged in
- boot
- hdmi works
- camera
--> camera doesn't work

(4)
- boot
- hdmi plugged in
- hdmi works
- camera
-> camera works


With a bit of brute-force [0] it seems the camera also works again even
with hdmi connected on boot. So conclusion would be that some clock
is misbehaving.

Now we'll "only" need to find out which one that is.


Heiko


[0]
Don't disable any clock gates

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 070dc47e95a1..8daf1fc3388c 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 
        set ^= enable;
 
+if (!enable)
+return;
+
        if (gate->lock)
                spin_lock_irqsave(gate->lock, flags);
        else



Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
> Hi Sebastian,
> 
> Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> > On 03.02.2021 20:54, Heiko Stübner wrote:
> > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> > >> I have tested your patch set on my nanoPC-T4, here is a complete log
> > >> with:
> > >> - relevant kernel log entries
> > >> - system information
> > >> - media ctl output
> > >> - sysfs entry information
> > >>
> > >> https://paste.debian.net/1183874/
> > >>
> > >> Additionally, to your patchset I have applied the following patches:
> > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> > >>
> > >> And just to not cause confusion the `media_dev` entries come from this
> > >> unmerged series:
> > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> > >>
> > >> I have actually been able to stream with both of my cameras at the same
> > >> time using the libcamera cam command.
> > >> I would like to thank you a lot for making this possible.
> > >
> > >Thanks for testing a dual camera setup. On my board I could only test
> > >the second ISP. And really glad it works for you tool :-) .
> > >
> > >Out of curiosity, do you also see that green tint in the images the cameras
> > >produce?
> > 
> > Yes, I do. Actually, I currently have two forms of a green tint, on my
> > OV13850 everything is quite dark and greenish, which is caused by the
> > missing 3A algorithms. On my OV4689, I have big patches of the image
> > with bright green color and flickering, I investigated if this is
> > connected to the 2nd ISP instance, but that doesn't seem to be the case
> > as I have the same results when I switch the CSI ports of the cameras.
> > 
> > I have found another issue, while testing I discovered following
> > issue:
> > When I start the system with an HDMI monitor connected, then the camera
> > on the 2nd port doesn't work. This is probably because the RX/TX is
> > reserved as a TX.
> > But it made me wonder because if the system has an RX, a TX, and
> > an RX/TX, why isn't the pure TX used by the monitor and the
> > cameras take RX and RX/TX?
> > Or do you think that this is maybe a malfunction of this patch?
> 
> I don't think it is an issue with this specific series, but still puzzling.
> 
> I.e. the DPHYs are actually only relevant to the DSI controllers,
> with TX0 being connected to DSI0 and TX1RX1 being connected
> to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
> 
> Out of curiosity what happens, when you boot without hdmi connected
> turn on the cameras, connect the hdmi after this, try the cameras again?
> 
> 
> Heiko
> 
> > 
> > >
> > >Thanks
> > >Heiko
> > 
> > Greetings,
> > Sebastian
> > 
> > >
> > >
> > >> If you like to you can add:
> > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> > >>
> > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> > >> >The rk3399 has two ISPs and right now only the first one is usable.
> > >> >The second ISP is connected to the TXRX dphy on the soc.
> > >> >
> > >> >The phy of ISP1 is only accessible through the DSI controller's
> > >> >io-memory, so this series adds support for simply using the dsi
> > >> >controller is a phy if needed.
> > >> >
> > >> >That solution is needed at least on rk3399 and rk3288 but no-one
> > >> >has looked at camera support on rk3288 at all, so right now
> > >> >only implement the rk3399 specifics.
> > >> >
> > >> >
> > >> >Heiko Stuebner (6):
> > >> >  drm/rockchip: dsi: add own additional pclk handling
> > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> > >> >  arm64: dts: rockchip: add isp1 node on rk3399
> > >> >
> > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> > >> > 4 files changed, 384 insertions(+)
> > >> >
> > >>
> > >
> > >
> > >
> > >
> > 
> 
>
Heiko Stübner Feb. 10, 2021, 11:15 a.m. UTC | #6
Hi Sebastian,

Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
> Hi Sebastian,
> 
> I did some tests myself today as well and can confirm your
> hdmi related finding - at least when plugged in on boot.
> 
> I tried some combinations of camera vs. hdmi and it seems
> really only when hdmi is plugged in on boot

as you can see in v2, it should work now even with hdmi
connected on boot. My patch ignored the grf-clock when
doing the grf-based init.

All clocks are on during boot and I guess the hdmi-driver
did disable it after its probe. The phy_power_on functions
did handle it correctly already, so it was only happening
with hdmi connected on boot.


Btw. do you plan on submitting your ov13850 driver
soonish?


Heiko


> 
> (1)
> - boot
> - camera
> --> works
> 
> (2)
> - boot
> - camera
> - hdmi plugged in
> - hdmi works
> - camera
> --> works
> 
> (3)
> - hdmi plugged in
> - boot
> - hdmi works
> - camera
> --> camera doesn't work
> 
> (4)
> - boot
> - hdmi plugged in
> - hdmi works
> - camera
> -> camera works
> 
> 
> With a bit of brute-force [0] it seems the camera also works again even
> with hdmi connected on boot. So conclusion would be that some clock
> is misbehaving.
> 
> Now we'll "only" need to find out which one that is.
> 
> 
> Heiko
> 
> 
> [0]
> Don't disable any clock gates
> 
> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> index 070dc47e95a1..8daf1fc3388c 100644
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
>  
>         set ^= enable;
>  
> +if (!enable)
> +return;
> +
>         if (gate->lock)
>                 spin_lock_irqsave(gate->lock, flags);
>         else
> 
> 
> 
> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
> > Hi Sebastian,
> > 
> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> > > On 03.02.2021 20:54, Heiko Stübner wrote:
> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
> > > >> with:
> > > >> - relevant kernel log entries
> > > >> - system information
> > > >> - media ctl output
> > > >> - sysfs entry information
> > > >>
> > > >> https://paste.debian.net/1183874/
> > > >>
> > > >> Additionally, to your patchset I have applied the following patches:
> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> > > >>
> > > >> And just to not cause confusion the `media_dev` entries come from this
> > > >> unmerged series:
> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> > > >>
> > > >> I have actually been able to stream with both of my cameras at the same
> > > >> time using the libcamera cam command.
> > > >> I would like to thank you a lot for making this possible.
> > > >
> > > >Thanks for testing a dual camera setup. On my board I could only test
> > > >the second ISP. And really glad it works for you tool :-) .
> > > >
> > > >Out of curiosity, do you also see that green tint in the images the cameras
> > > >produce?
> > > 
> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
> > > OV13850 everything is quite dark and greenish, which is caused by the
> > > missing 3A algorithms. On my OV4689, I have big patches of the image
> > > with bright green color and flickering, I investigated if this is
> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
> > > as I have the same results when I switch the CSI ports of the cameras.
> > > 
> > > I have found another issue, while testing I discovered following
> > > issue:
> > > When I start the system with an HDMI monitor connected, then the camera
> > > on the 2nd port doesn't work. This is probably because the RX/TX is
> > > reserved as a TX.
> > > But it made me wonder because if the system has an RX, a TX, and
> > > an RX/TX, why isn't the pure TX used by the monitor and the
> > > cameras take RX and RX/TX?
> > > Or do you think that this is maybe a malfunction of this patch?
> > 
> > I don't think it is an issue with this specific series, but still puzzling.
> > 
> > I.e. the DPHYs are actually only relevant to the DSI controllers,
> > with TX0 being connected to DSI0 and TX1RX1 being connected
> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
> > 
> > Out of curiosity what happens, when you boot without hdmi connected
> > turn on the cameras, connect the hdmi after this, try the cameras again?
> > 
> > 
> > Heiko
> > 
> > > 
> > > >
> > > >Thanks
> > > >Heiko
> > > 
> > > Greetings,
> > > Sebastian
> > > 
> > > >
> > > >
> > > >> If you like to you can add:
> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> > > >>
> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
> > > >> >The second ISP is connected to the TXRX dphy on the soc.
> > > >> >
> > > >> >The phy of ISP1 is only accessible through the DSI controller's
> > > >> >io-memory, so this series adds support for simply using the dsi
> > > >> >controller is a phy if needed.
> > > >> >
> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
> > > >> >has looked at camera support on rk3288 at all, so right now
> > > >> >only implement the rk3399 specifics.
> > > >> >
> > > >> >
> > > >> >Heiko Stuebner (6):
> > > >> >  drm/rockchip: dsi: add own additional pclk handling
> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
> > > >> >
> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> > > >> > 4 files changed, 384 insertions(+)
> > > >> >
> > > >>
> > > >
> > > >
> > > >
> > > >
> > > 
> > 
> > 
> 
>
Sebastian Fricke Feb. 11, 2021, 5:25 a.m. UTC | #7
Hey Heiko,

On 10.02.2021 12:15, Heiko Stübner wrote:
>Hi Sebastian,
>
>Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
>> Hi Sebastian,
>>
>> I did some tests myself today as well and can confirm your
>> hdmi related finding - at least when plugged in on boot.
>>
>> I tried some combinations of camera vs. hdmi and it seems
>> really only when hdmi is plugged in on boot
>
>as you can see in v2, it should work now even with hdmi
>connected on boot. My patch ignored the grf-clock when
>doing the grf-based init.
>
>All clocks are on during boot and I guess the hdmi-driver
>did disable it after its probe. The phy_power_on functions
>did handle it correctly already, so it was only happening
>with hdmi connected on boot.

Thank you very much for solving that problem, I've tested the scenarios
described below and it works like a charm. (With your V2)
>
>
>Btw. do you plan on submitting your ov13850 driver
>soonish?

Actually, I have posted the patch already see here:
https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/

I currently review the requested changes and questions and will soon
post a second version, but I expect quite some time until it is actually
merged.

>
>
>Heiko

Greetings,
Sebastian

>
>
>>
>> (1)
>> - boot
>> - camera
>> --> works
>>
>> (2)
>> - boot
>> - camera
>> - hdmi plugged in
>> - hdmi works
>> - camera
>> --> works
>>
>> (3)
>> - hdmi plugged in
>> - boot
>> - hdmi works
>> - camera
>> --> camera doesn't work
>>
>> (4)
>> - boot
>> - hdmi plugged in
>> - hdmi works
>> - camera
>> -> camera works
>>
>>
>> With a bit of brute-force [0] it seems the camera also works again even
>> with hdmi connected on boot. So conclusion would be that some clock
>> is misbehaving.
>>
>> Now we'll "only" need to find out which one that is.
>>
>>
>> Heiko
>>
>>
>> [0]
>> Don't disable any clock gates
>>
>> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
>> index 070dc47e95a1..8daf1fc3388c 100644
>> --- a/drivers/clk/clk-gate.c
>> +++ b/drivers/clk/clk-gate.c
>> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
>>
>>         set ^= enable;
>>
>> +if (!enable)
>> +return;
>> +
>>         if (gate->lock)
>>                 spin_lock_irqsave(gate->lock, flags);
>>         else
>>
>>
>>
>> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
>> > Hi Sebastian,
>> >
>> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
>> > > On 03.02.2021 20:54, Heiko Stübner wrote:
>> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
>> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
>> > > >> with:
>> > > >> - relevant kernel log entries
>> > > >> - system information
>> > > >> - media ctl output
>> > > >> - sysfs entry information
>> > > >>
>> > > >> https://paste.debian.net/1183874/
>> > > >>
>> > > >> Additionally, to your patchset I have applied the following patches:
>> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
>> > > >>
>> > > >> And just to not cause confusion the `media_dev` entries come from this
>> > > >> unmerged series:
>> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
>> > > >>
>> > > >> I have actually been able to stream with both of my cameras at the same
>> > > >> time using the libcamera cam command.
>> > > >> I would like to thank you a lot for making this possible.
>> > > >
>> > > >Thanks for testing a dual camera setup. On my board I could only test
>> > > >the second ISP. And really glad it works for you tool :-) .
>> > > >
>> > > >Out of curiosity, do you also see that green tint in the images the cameras
>> > > >produce?
>> > >
>> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
>> > > OV13850 everything is quite dark and greenish, which is caused by the
>> > > missing 3A algorithms. On my OV4689, I have big patches of the image
>> > > with bright green color and flickering, I investigated if this is
>> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
>> > > as I have the same results when I switch the CSI ports of the cameras.
>> > >
>> > > I have found another issue, while testing I discovered following
>> > > issue:
>> > > When I start the system with an HDMI monitor connected, then the camera
>> > > on the 2nd port doesn't work. This is probably because the RX/TX is
>> > > reserved as a TX.
>> > > But it made me wonder because if the system has an RX, a TX, and
>> > > an RX/TX, why isn't the pure TX used by the monitor and the
>> > > cameras take RX and RX/TX?
>> > > Or do you think that this is maybe a malfunction of this patch?
>> >
>> > I don't think it is an issue with this specific series, but still puzzling.
>> >
>> > I.e. the DPHYs are actually only relevant to the DSI controllers,
>> > with TX0 being connected to DSI0 and TX1RX1 being connected
>> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
>> >
>> > Out of curiosity what happens, when you boot without hdmi connected
>> > turn on the cameras, connect the hdmi after this, try the cameras again?
>> >
>> >
>> > Heiko
>> >
>> > >
>> > > >
>> > > >Thanks
>> > > >Heiko
>> > >
>> > > Greetings,
>> > > Sebastian
>> > >
>> > > >
>> > > >
>> > > >> If you like to you can add:
>> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
>> > > >>
>> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
>> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
>> > > >> >The second ISP is connected to the TXRX dphy on the soc.
>> > > >> >
>> > > >> >The phy of ISP1 is only accessible through the DSI controller's
>> > > >> >io-memory, so this series adds support for simply using the dsi
>> > > >> >controller is a phy if needed.
>> > > >> >
>> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
>> > > >> >has looked at camera support on rk3288 at all, so right now
>> > > >> >only implement the rk3399 specifics.
>> > > >> >
>> > > >> >
>> > > >> >Heiko Stuebner (6):
>> > > >> >  drm/rockchip: dsi: add own additional pclk handling
>> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
>> > > >> >
>> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
>> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
>> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
>> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
>> > > >> > 4 files changed, 384 insertions(+)
>> > > >> >
>> > > >>
>> > > >
>> > > >
>> > > >
>> > > >
>> > >
>> >
>> >
>>
>>
>
>
>
>
Heiko Stübner Feb. 11, 2021, 2:42 p.m. UTC | #8
Hi Sebastian,

Am Donnerstag, 11. Februar 2021, 06:25:15 CET schrieb Sebastian Fricke:
> Hey Heiko,
> 
> On 10.02.2021 12:15, Heiko Stübner wrote:
> >Hi Sebastian,
> >
> >Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
> >> Hi Sebastian,
> >>
> >> I did some tests myself today as well and can confirm your
> >> hdmi related finding - at least when plugged in on boot.
> >>
> >> I tried some combinations of camera vs. hdmi and it seems
> >> really only when hdmi is plugged in on boot
> >
> >as you can see in v2, it should work now even with hdmi
> >connected on boot. My patch ignored the grf-clock when
> >doing the grf-based init.
> >
> >All clocks are on during boot and I guess the hdmi-driver
> >did disable it after its probe. The phy_power_on functions
> >did handle it correctly already, so it was only happening
> >with hdmi connected on boot.
> 
> Thank you very much for solving that problem, I've tested the scenarios
> described below and it works like a charm. (With your V2)
> >
> >
> >Btw. do you plan on submitting your ov13850 driver
> >soonish?
> 
> Actually, I have posted the patch already see here:
> https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/

very cool to see

> I currently review the requested changes and questions and will soon
> post a second version, but I expect quite some time until it is actually
> merged.

could you Cc me on future versions?


Thanks
Heiko
> 
> Greetings,
> Sebastian
> 
> >
> >
> >>
> >> (1)
> >> - boot
> >> - camera
> >> --> works
> >>
> >> (2)
> >> - boot
> >> - camera
> >> - hdmi plugged in
> >> - hdmi works
> >> - camera
> >> --> works
> >>
> >> (3)
> >> - hdmi plugged in
> >> - boot
> >> - hdmi works
> >> - camera
> >> --> camera doesn't work
> >>
> >> (4)
> >> - boot
> >> - hdmi plugged in
> >> - hdmi works
> >> - camera
> >> -> camera works
> >>
> >>
> >> With a bit of brute-force [0] it seems the camera also works again even
> >> with hdmi connected on boot. So conclusion would be that some clock
> >> is misbehaving.
> >>
> >> Now we'll "only" need to find out which one that is.
> >>
> >>
> >> Heiko
> >>
> >>
> >> [0]
> >> Don't disable any clock gates
> >>
> >> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> >> index 070dc47e95a1..8daf1fc3388c 100644
> >> --- a/drivers/clk/clk-gate.c
> >> +++ b/drivers/clk/clk-gate.c
> >> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
> >>
> >>         set ^= enable;
> >>
> >> +if (!enable)
> >> +return;
> >> +
> >>         if (gate->lock)
> >>                 spin_lock_irqsave(gate->lock, flags);
> >>         else
> >>
> >>
> >>
> >> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
> >> > Hi Sebastian,
> >> >
> >> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
> >> > > On 03.02.2021 20:54, Heiko Stübner wrote:
> >> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
> >> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
> >> > > >> with:
> >> > > >> - relevant kernel log entries
> >> > > >> - system information
> >> > > >> - media ctl output
> >> > > >> - sysfs entry information
> >> > > >>
> >> > > >> https://paste.debian.net/1183874/
> >> > > >>
> >> > > >> Additionally, to your patchset I have applied the following patches:
> >> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
> >> > > >>
> >> > > >> And just to not cause confusion the `media_dev` entries come from this
> >> > > >> unmerged series:
> >> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
> >> > > >>
> >> > > >> I have actually been able to stream with both of my cameras at the same
> >> > > >> time using the libcamera cam command.
> >> > > >> I would like to thank you a lot for making this possible.
> >> > > >
> >> > > >Thanks for testing a dual camera setup. On my board I could only test
> >> > > >the second ISP. And really glad it works for you tool :-) .
> >> > > >
> >> > > >Out of curiosity, do you also see that green tint in the images the cameras
> >> > > >produce?
> >> > >
> >> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
> >> > > OV13850 everything is quite dark and greenish, which is caused by the
> >> > > missing 3A algorithms. On my OV4689, I have big patches of the image
> >> > > with bright green color and flickering, I investigated if this is
> >> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
> >> > > as I have the same results when I switch the CSI ports of the cameras.
> >> > >
> >> > > I have found another issue, while testing I discovered following
> >> > > issue:
> >> > > When I start the system with an HDMI monitor connected, then the camera
> >> > > on the 2nd port doesn't work. This is probably because the RX/TX is
> >> > > reserved as a TX.
> >> > > But it made me wonder because if the system has an RX, a TX, and
> >> > > an RX/TX, why isn't the pure TX used by the monitor and the
> >> > > cameras take RX and RX/TX?
> >> > > Or do you think that this is maybe a malfunction of this patch?
> >> >
> >> > I don't think it is an issue with this specific series, but still puzzling.
> >> >
> >> > I.e. the DPHYs are actually only relevant to the DSI controllers,
> >> > with TX0 being connected to DSI0 and TX1RX1 being connected
> >> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
> >> >
> >> > Out of curiosity what happens, when you boot without hdmi connected
> >> > turn on the cameras, connect the hdmi after this, try the cameras again?
> >> >
> >> >
> >> > Heiko
> >> >
> >> > >
> >> > > >
> >> > > >Thanks
> >> > > >Heiko
> >> > >
> >> > > Greetings,
> >> > > Sebastian
> >> > >
> >> > > >
> >> > > >
> >> > > >> If you like to you can add:
> >> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> >> > > >>
> >> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
> >> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
> >> > > >> >The second ISP is connected to the TXRX dphy on the soc.
> >> > > >> >
> >> > > >> >The phy of ISP1 is only accessible through the DSI controller's
> >> > > >> >io-memory, so this series adds support for simply using the dsi
> >> > > >> >controller is a phy if needed.
> >> > > >> >
> >> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
> >> > > >> >has looked at camera support on rk3288 at all, so right now
> >> > > >> >only implement the rk3399 specifics.
> >> > > >> >
> >> > > >> >
> >> > > >> >Heiko Stuebner (6):
> >> > > >> >  drm/rockchip: dsi: add own additional pclk handling
> >> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
> >> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
> >> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
> >> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
> >> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
> >> > > >> >
> >> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
> >> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
> >> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
> >> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
> >> > > >> > 4 files changed, 384 insertions(+)
> >> > > >> >
> >> > > >>
> >> > > >
> >> > > >
> >> > > >
> >> > > >
> >> > >
> >> >
> >> >
> >>
> >>
> >
> >
> >
> >
>
Sebastian Fricke Feb. 13, 2021, 11:19 a.m. UTC | #9
Hey Heiko,

On 11.02.2021 15:42, Heiko Stübner wrote:
>Hi Sebastian,
>
>Am Donnerstag, 11. Februar 2021, 06:25:15 CET schrieb Sebastian Fricke:
>> Hey Heiko,
>>
>> On 10.02.2021 12:15, Heiko Stübner wrote:
>> >Hi Sebastian,
>> >
>> >Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
>> >> Hi Sebastian,
>> >>
>> >> I did some tests myself today as well and can confirm your
>> >> hdmi related finding - at least when plugged in on boot.
>> >>
>> >> I tried some combinations of camera vs. hdmi and it seems
>> >> really only when hdmi is plugged in on boot
>> >
>> >as you can see in v2, it should work now even with hdmi
>> >connected on boot. My patch ignored the grf-clock when
>> >doing the grf-based init.
>> >
>> >All clocks are on during boot and I guess the hdmi-driver
>> >did disable it after its probe. The phy_power_on functions
>> >did handle it correctly already, so it was only happening
>> >with hdmi connected on boot.
>>
>> Thank you very much for solving that problem, I've tested the scenarios
>> described below and it works like a charm. (With your V2)
>> >
>> >
>> >Btw. do you plan on submitting your ov13850 driver
>> >soonish?
>>
>> Actually, I have posted the patch already see here:
>> https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/
>
>very cool to see
>
>> I currently review the requested changes and questions and will soon
>> post a second version, but I expect quite some time until it is actually
>> merged.
>
>could you Cc me on future versions?

Sure will do :)

>
>
>Thanks
>Heiko

Sebastian
>>
>> Greetings,
>> Sebastian
>>
>> >
>> >
>> >>
>> >> (1)
>> >> - boot
>> >> - camera
>> >> --> works
>> >>
>> >> (2)
>> >> - boot
>> >> - camera
>> >> - hdmi plugged in
>> >> - hdmi works
>> >> - camera
>> >> --> works
>> >>
>> >> (3)
>> >> - hdmi plugged in
>> >> - boot
>> >> - hdmi works
>> >> - camera
>> >> --> camera doesn't work
>> >>
>> >> (4)
>> >> - boot
>> >> - hdmi plugged in
>> >> - hdmi works
>> >> - camera
>> >> -> camera works
>> >>
>> >>
>> >> With a bit of brute-force [0] it seems the camera also works again even
>> >> with hdmi connected on boot. So conclusion would be that some clock
>> >> is misbehaving.
>> >>
>> >> Now we'll "only" need to find out which one that is.
>> >>
>> >>
>> >> Heiko
>> >>
>> >>
>> >> [0]
>> >> Don't disable any clock gates
>> >>
>> >> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
>> >> index 070dc47e95a1..8daf1fc3388c 100644
>> >> --- a/drivers/clk/clk-gate.c
>> >> +++ b/drivers/clk/clk-gate.c
>> >> @@ -61,6 +61,9 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
>> >>
>> >>         set ^= enable;
>> >>
>> >> +if (!enable)
>> >> +return;
>> >> +
>> >>         if (gate->lock)
>> >>                 spin_lock_irqsave(gate->lock, flags);
>> >>         else
>> >>
>> >>
>> >>
>> >> Am Freitag, 5. Februar 2021, 09:15:47 CET schrieb Heiko Stübner:
>> >> > Hi Sebastian,
>> >> >
>> >> > Am Freitag, 5. Februar 2021, 07:43:35 CET schrieb Sebastian Fricke:
>> >> > > On 03.02.2021 20:54, Heiko Stübner wrote:
>> >> > > >Am Mittwoch, 3. Februar 2021, 19:14:22 CET schrieb Sebastian Fricke:
>> >> > > >> I have tested your patch set on my nanoPC-T4, here is a complete log
>> >> > > >> with:
>> >> > > >> - relevant kernel log entries
>> >> > > >> - system information
>> >> > > >> - media ctl output
>> >> > > >> - sysfs entry information
>> >> > > >>
>> >> > > >> https://paste.debian.net/1183874/
>> >> > > >>
>> >> > > >> Additionally, to your patchset I have applied the following patches:
>> >> > > >> https://github.com/initBasti/Linux_kernel_media_tree_fork/commits/dual_cam_setup
>> >> > > >>
>> >> > > >> And just to not cause confusion the `media_dev` entries come from this
>> >> > > >> unmerged series:
>> >> > > >> https://patchwork.kernel.org/project/linux-media/list/?series=426269
>> >> > > >>
>> >> > > >> I have actually been able to stream with both of my cameras at the same
>> >> > > >> time using the libcamera cam command.
>> >> > > >> I would like to thank you a lot for making this possible.
>> >> > > >
>> >> > > >Thanks for testing a dual camera setup. On my board I could only test
>> >> > > >the second ISP. And really glad it works for you tool :-) .
>> >> > > >
>> >> > > >Out of curiosity, do you also see that green tint in the images the cameras
>> >> > > >produce?
>> >> > >
>> >> > > Yes, I do. Actually, I currently have two forms of a green tint, on my
>> >> > > OV13850 everything is quite dark and greenish, which is caused by the
>> >> > > missing 3A algorithms. On my OV4689, I have big patches of the image
>> >> > > with bright green color and flickering, I investigated if this is
>> >> > > connected to the 2nd ISP instance, but that doesn't seem to be the case
>> >> > > as I have the same results when I switch the CSI ports of the cameras.
>> >> > >
>> >> > > I have found another issue, while testing I discovered following
>> >> > > issue:
>> >> > > When I start the system with an HDMI monitor connected, then the camera
>> >> > > on the 2nd port doesn't work. This is probably because the RX/TX is
>> >> > > reserved as a TX.
>> >> > > But it made me wonder because if the system has an RX, a TX, and
>> >> > > an RX/TX, why isn't the pure TX used by the monitor and the
>> >> > > cameras take RX and RX/TX?
>> >> > > Or do you think that this is maybe a malfunction of this patch?
>> >> >
>> >> > I don't think it is an issue with this specific series, but still puzzling.
>> >> >
>> >> > I.e. the DPHYs are actually only relevant to the DSI controllers,
>> >> > with TX0 being connected to DSI0 and TX1RX1 being connected
>> >> > to DSI1. So having an hdmi display _in theory_ shouldn't matter at all.
>> >> >
>> >> > Out of curiosity what happens, when you boot without hdmi connected
>> >> > turn on the cameras, connect the hdmi after this, try the cameras again?
>> >> >
>> >> >
>> >> > Heiko
>> >> >
>> >> > >
>> >> > > >
>> >> > > >Thanks
>> >> > > >Heiko
>> >> > >
>> >> > > Greetings,
>> >> > > Sebastian
>> >> > >
>> >> > > >
>> >> > > >
>> >> > > >> If you like to you can add:
>> >> > > >> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
>> >> > > >>
>> >> > > >> On 02.02.2021 15:56, Heiko Stuebner wrote:
>> >> > > >> >The rk3399 has two ISPs and right now only the first one is usable.
>> >> > > >> >The second ISP is connected to the TXRX dphy on the soc.
>> >> > > >> >
>> >> > > >> >The phy of ISP1 is only accessible through the DSI controller's
>> >> > > >> >io-memory, so this series adds support for simply using the dsi
>> >> > > >> >controller is a phy if needed.
>> >> > > >> >
>> >> > > >> >That solution is needed at least on rk3399 and rk3288 but no-one
>> >> > > >> >has looked at camera support on rk3288 at all, so right now
>> >> > > >> >only implement the rk3399 specifics.
>> >> > > >> >
>> >> > > >> >
>> >> > > >> >Heiko Stuebner (6):
>> >> > > >> >  drm/rockchip: dsi: add own additional pclk handling
>> >> > > >> >  dt-bindings: display: rockchip-dsi: add optional #phy-cells property
>> >> > > >> >  drm/rockchip: dsi: add ability to work as a phy instead of full dsi
>> >> > > >> >  arm64: dts: rockchip: add #phy-cells to mipi-dsi1
>> >> > > >> >  arm64: dts: rockchip: add cif clk-control pinctrl for rk3399
>> >> > > >> >  arm64: dts: rockchip: add isp1 node on rk3399
>> >> > > >> >
>> >> > > >> > .../display/rockchip/dw_mipi_dsi_rockchip.txt |   1 +
>> >> > > >> > arch/arm64/boot/dts/rockchip/rk3399.dtsi      |  39 ++
>> >> > > >> > drivers/gpu/drm/rockchip/Kconfig              |   2 +
>> >> > > >> > .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 342 ++++++++++++++++++
>> >> > > >> > 4 files changed, 384 insertions(+)
>> >> > > >> >
>> >> > > >>
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > >
>> >> >
>> >> >
>> >>
>> >>
>> >
>> >
>> >
>> >
>>
>
>
>
>
Heiko Stübner Feb. 13, 2021, 10:33 p.m. UTC | #10
Hi Sebastian,

Am Samstag, 13. Februar 2021, 12:19:57 CET schrieb Sebastian Fricke:
> On 11.02.2021 15:42, Heiko Stübner wrote:
> >Am Donnerstag, 11. Februar 2021, 06:25:15 CET schrieb Sebastian Fricke:
> >> On 10.02.2021 12:15, Heiko Stübner wrote:
> >> >Am Freitag, 5. Februar 2021, 15:55:56 CET schrieb Heiko Stübner:
> >> >> I did some tests myself today as well and can confirm your
> >> >> hdmi related finding - at least when plugged in on boot.
> >> >>
> >> >> I tried some combinations of camera vs. hdmi and it seems
> >> >> really only when hdmi is plugged in on boot
> >> >
> >> >as you can see in v2, it should work now even with hdmi
> >> >connected on boot. My patch ignored the grf-clock when
> >> >doing the grf-based init.
> >> >
> >> >All clocks are on during boot and I guess the hdmi-driver
> >> >did disable it after its probe. The phy_power_on functions
> >> >did handle it correctly already, so it was only happening
> >> >with hdmi connected on boot.
> >>
> >> Thank you very much for solving that problem, I've tested the scenarios
> >> described below and it works like a charm. (With your V2)
> >> >
> >> >
> >> >Btw. do you plan on submitting your ov13850 driver
> >> >soonish?
> >>
> >> Actually, I have posted the patch already see here:
> >> https://patchwork.kernel.org/project/linux-media/patch/20210130182313.32903-2-sebastian.fricke@posteo.net/
> >
> >very cool to see
> >
> >> I currently review the requested changes and questions and will soon
> >> post a second version, but I expect quite some time until it is actually
> >> merged.
> >
> >could you Cc me on future versions?
> 
> Sure will do :)

by the way, you could also answer the v2 series with a

Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>

so we get some coverage :-)

Thanks
Heiko