Re: [PATCH v6 3/3] drm/rockchip: Add basic RK3588 HDMI output support

2024-09-10 Thread Heiko Stübner
Am Dienstag, 10. September 2024, 18:39:54 CEST schrieb Heiko Stübner:
> Am Dienstag, 10. September 2024, 17:41:42 CEST schrieb Cristian Ciocaltea:
> > On 9/10/24 6:21 PM, Heiko Stübner wrote:
> > > Am Dienstag, 10. September 2024, 17:07:57 CEST schrieb Heiko Stübner:
> > >> Am Freitag, 6. September 2024, 03:17:42 CEST schrieb Cristian Ciocaltea:
> > >>> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1
> > >>> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a
> > >>> Samsung IP block.
> > >>>
> > >>> Add just the basic support for now, i.e. RGB output up to 4K@60Hz,
> > >>> without audio, CEC or any of the HDMI 2.1 specific features.
> > >>>
> > >>> Co-developed-by: Algea Cao 
> > >>> Signed-off-by: Algea Cao 
> > >>> Tested-by: Heiko Stuebner 
> > >>> Signed-off-by: Cristian Ciocaltea 
> > >>
> > >> I had switched from the v3 to this v6 in my playground-kernel today,
> > >> with v3 I've never seen those, but now with v6 I have gotten multiple
> > >> times:
> > >>
> > >> [  805.730608] Internal error: synchronous external abort: 
> > >> 9610 [#1] PREEMPT SMP
> > >> [  805.739764] Modules linked in: snd_soc_simple_card crct10dif_ce 
> > >> snd_soc_simple_card_utils panthor drm_gpuvm drm_exec fuse
> > >> [  805.752031] CPU: 3 UID: 0 PID: 775 Comm: Xorg Not tainted 
> > >> 6.11.0-rc7-00099-g459302f1f908-dirty #934
> > >> [  805.762143] Hardware name: Theobroma Systems RK3588-Q7 SoM on Haikou 
> > >> devkit (DT)
> > >> [  805.770407] pstate: 204000c9 (nzCv daIF +PAN -UAO -TCO -DIT -SSBS 
> > >> BTYPE=--)
> > >> [  805.778186] pc : regmap_mmio_read32le+0x8/0x20
> > >> [  805.783155] lr : regmap_mmio_read+0x44/0x70
> > >> [  805.787828] sp : 80008293b830
> > >> [  805.791516] x29: 80008293b830 x28: 80008293bce8 x27: 
> > >> 0001f20ab080
> > >> [  805.799495] x26: 800081139500 x25:  x24: 
> > >> 0010
> > >> [  805.807472] x23:  x22: 0001f5a4b400 x21: 
> > >> 80008293b8c4
> > >> [  805.815450] x20: 0968 x19: 0001f5a27a80 x18: 
> > >> 0070
> > >> [  805.823428] x17: 000244140005 x16: 04650441043c x15: 
> > >> 043808980804
> > >> [  805.831406] x14: 07d8089807800780 x13: 043808980804 x12: 
> > >> 800081133630
> > >> [  805.839384] x11: 000244140005 x10: 04650441043c x9 : 
> > >> 800081a59000
> > >> [  805.847361] x8 : 07d8089807800780 x7 :  x6 : 
> > >> 0001f5b453c0
> > >> [  805.855339] x5 : 800080750dc0 x4 : 0968 x3 : 
> > >> 0968
> > >> [  805.863316] x2 : 800080751520 x1 : 0968 x0 : 
> > >> 800083b20968
> > >> [  805.871294] Call trace:
> > >> [  805.874012]  regmap_mmio_read32le+0x8/0x20
> > >> [  805.878588]  _regmap_bus_reg_read+0x6c/0xac
> > >> [  805.883262]  _regmap_read+0x60/0xd8
> > >> [  805.887159]  _regmap_update_bits+0xf4/0x140
> > >> [  805.891832]  regmap_update_bits_base+0x64/0xa0
> > >> [  805.896797]  dw_hdmi_qp_bridge_atomic_enable+0x134/0x220
> > >> [  805.902734]  drm_atomic_bridge_chain_enable+0x54/0xc8
> > >> [  805.908380]  drm_atomic_helper_commit_modeset_enables+0x194/0x280
> > >> [  805.915190]  drm_atomic_helper_commit_tail_rpm+0x50/0xa0
> > >> [  805.921125]  commit_tail+0xa0/0x1a0
> > >> [  805.925021]  drm_atomic_helper_commit+0x17c/0x1b0
> > >> [  805.930276]  drm_atomic_commit+0xb8/0x100
> > >> [  805.934754]  drm_atomic_connector_commit_dpms+0xe0/0x110
> > >> [  805.940690]  drm_mode_obj_set_property_ioctl+0x1c0/0x420
> > >> [  805.946626]  drm_connector_property_set_ioctl+0x3c/0x68
> > >> [  805.952465]  drm_ioctl_kernel+0xc0/0x130
> > >> [  805.956846]  drm_ioctl+0x214/0x4a0
> > >> [  805.960643]  __arm64_sys_ioctl+0xac/0xf8
> > >> [  805.965025]  invoke_syscall+0x48/0x104
> > >> [  805.969214]  el0_svc_common.constprop.0+0x40/0xe0
> > >> [  805.974470]  do_el0_svc+0x1c/0x28
> > >> [  805.978171]  el0_svc+0x34/0xe0
> > >> [  805.981582]  el0t_64_sync_handler+0x120/0x12c
> > >> [  805.

Re: [PATCH v6 3/3] drm/rockchip: Add basic RK3588 HDMI output support

2024-09-10 Thread Heiko Stübner
Am Freitag, 6. September 2024, 03:17:42 CEST schrieb Cristian Ciocaltea:
> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1
> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a
> Samsung IP block.
> 
> Add just the basic support for now, i.e. RGB output up to 4K@60Hz,
> without audio, CEC or any of the HDMI 2.1 specific features.
> 
> Co-developed-by: Algea Cao 
> Signed-off-by: Algea Cao 
> Tested-by: Heiko Stuebner 
> Signed-off-by: Cristian Ciocaltea 

[...]

> diff --git a/drivers/gpu/drm/rockchip/Makefile 
> b/drivers/gpu/drm/rockchip/Makefile
> index 3ff7b21c0414..3eab662a5a1d 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -11,6 +11,7 @@ rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o 
> rockchip_vop_reg.o

> +static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device 
> *master,
> + void *data)
> +{
> + static const char * const clk_names[] = {
> + "pclk", "earc", "aud", "hdp", "hclk_vo1",
> + "ref" /* keep "ref" last */
> + };

[...]

> + for (i = 0; i < ARRAY_SIZE(clk_names); i++) {
> + clk = devm_clk_get_enabled(hdmi->dev, clk_names[i]);
> +
> + if (IS_ERR(clk)) {
> + ret = PTR_ERR(clk);
> + if (ret != -EPROBE_DEFER)
> + drm_err(hdmi, "Failed to get %s clock: %d\n",
> + clk_names[i], ret);
> + return ret;
> + }
> + }
> + hdmi->ref_clk = clk;

How about using devm_clk_bulk_get_all_enable() for everything except the
refclk and a separate call to devm_clk_get_enabled() for that refclk .

That hdmi->ref_clk just accidentially falls out of that loop at the end
looks somewhat strange, so getting and keeping that refclk
separately would make this look cleaner.




Re: [PATCH v6 3/3] drm/rockchip: Add basic RK3588 HDMI output support

2024-09-10 Thread Heiko Stübner
Am Dienstag, 10. September 2024, 17:41:42 CEST schrieb Cristian Ciocaltea:
> On 9/10/24 6:21 PM, Heiko Stübner wrote:
> > Am Dienstag, 10. September 2024, 17:07:57 CEST schrieb Heiko Stübner:
> >> Am Freitag, 6. September 2024, 03:17:42 CEST schrieb Cristian Ciocaltea:
> >>> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1
> >>> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a
> >>> Samsung IP block.
> >>>
> >>> Add just the basic support for now, i.e. RGB output up to 4K@60Hz,
> >>> without audio, CEC or any of the HDMI 2.1 specific features.
> >>>
> >>> Co-developed-by: Algea Cao 
> >>> Signed-off-by: Algea Cao 
> >>> Tested-by: Heiko Stuebner 
> >>> Signed-off-by: Cristian Ciocaltea 
> >>
> >> I had switched from the v3 to this v6 in my playground-kernel today,
> >> with v3 I've never seen those, but now with v6 I have gotten multiple
> >> times:
> >>
> >> [  805.730608] Internal error: synchronous external abort: 
> >> 9610 [#1] PREEMPT SMP
> >> [  805.739764] Modules linked in: snd_soc_simple_card crct10dif_ce 
> >> snd_soc_simple_card_utils panthor drm_gpuvm drm_exec fuse
> >> [  805.752031] CPU: 3 UID: 0 PID: 775 Comm: Xorg Not tainted 
> >> 6.11.0-rc7-00099-g459302f1f908-dirty #934
> >> [  805.762143] Hardware name: Theobroma Systems RK3588-Q7 SoM on Haikou 
> >> devkit (DT)
> >> [  805.770407] pstate: 204000c9 (nzCv daIF +PAN -UAO -TCO -DIT -SSBS 
> >> BTYPE=--)
> >> [  805.778186] pc : regmap_mmio_read32le+0x8/0x20
> >> [  805.783155] lr : regmap_mmio_read+0x44/0x70
> >> [  805.787828] sp : 80008293b830
> >> [  805.791516] x29: 80008293b830 x28: 80008293bce8 x27: 
> >> 0001f20ab080
> >> [  805.799495] x26: 800081139500 x25:  x24: 
> >> 0010
> >> [  805.807472] x23:  x22: 0001f5a4b400 x21: 
> >> 80008293b8c4
> >> [  805.815450] x20: 0968 x19: 0001f5a27a80 x18: 
> >> 0070
> >> [  805.823428] x17: 000244140005 x16: 04650441043c x15: 
> >> 043808980804
> >> [  805.831406] x14: 07d8089807800780 x13: 043808980804 x12: 
> >> 800081133630
> >> [  805.839384] x11: 000244140005 x10: 04650441043c x9 : 
> >> 800081a59000
> >> [  805.847361] x8 : 07d8089807800780 x7 :  x6 : 
> >> 0001f5b453c0
> >> [  805.855339] x5 : 800080750dc0 x4 : 0968 x3 : 
> >> 0968
> >> [  805.863316] x2 : 800080751520 x1 : 0968 x0 : 
> >> 800083b20968
> >> [  805.871294] Call trace:
> >> [  805.874012]  regmap_mmio_read32le+0x8/0x20
> >> [  805.878588]  _regmap_bus_reg_read+0x6c/0xac
> >> [  805.883262]  _regmap_read+0x60/0xd8
> >> [  805.887159]  _regmap_update_bits+0xf4/0x140
> >> [  805.891832]  regmap_update_bits_base+0x64/0xa0
> >> [  805.896797]  dw_hdmi_qp_bridge_atomic_enable+0x134/0x220
> >> [  805.902734]  drm_atomic_bridge_chain_enable+0x54/0xc8
> >> [  805.908380]  drm_atomic_helper_commit_modeset_enables+0x194/0x280
> >> [  805.915190]  drm_atomic_helper_commit_tail_rpm+0x50/0xa0
> >> [  805.921125]  commit_tail+0xa0/0x1a0
> >> [  805.925021]  drm_atomic_helper_commit+0x17c/0x1b0
> >> [  805.930276]  drm_atomic_commit+0xb8/0x100
> >> [  805.934754]  drm_atomic_connector_commit_dpms+0xe0/0x110
> >> [  805.940690]  drm_mode_obj_set_property_ioctl+0x1c0/0x420
> >> [  805.946626]  drm_connector_property_set_ioctl+0x3c/0x68
> >> [  805.952465]  drm_ioctl_kernel+0xc0/0x130
> >> [  805.956846]  drm_ioctl+0x214/0x4a0
> >> [  805.960643]  __arm64_sys_ioctl+0xac/0xf8
> >> [  805.965025]  invoke_syscall+0x48/0x104
> >> [  805.969214]  el0_svc_common.constprop.0+0x40/0xe0
> >> [  805.974470]  do_el0_svc+0x1c/0x28
> >> [  805.978171]  el0_svc+0x34/0xe0
> >> [  805.981582]  el0t_64_sync_handler+0x120/0x12c
> >> [  805.986449]  el0t_64_sync+0x190/0x194
> >> [  805.990540] Code: d503201f d503201f f940 8b214000 (b940)
> >>
> >> I guess that might be some clocking issue?
> > 
> > Forgot to add, this happens when the display has blanked and then is
> > supposed to unblank again.
> 
> Hmm, I've never encountered this while testing with my v6.11-rc1 based
> tree.  What is your current kernel base?  Did you change it while
> switching from v3 to v6?
> 
> I'll rebase my tree onto latest linux-next and see if I can reproduce.

The setup is 6.11-rc7 with your hdmi series + my wip dsi + X11 running
on top.

At some point after being idle a while this blanks the display, which will
probably turn off clocks and such. After moving the mouse or just
doing anything else that unblanks the display, that splat happens.

Apart from updating mesa from 24.2.0 to 24.2.2 I haven't changed
anything in my test-setup so far.




Re: [PATCH v6 3/3] drm/rockchip: Add basic RK3588 HDMI output support

2024-09-10 Thread Heiko Stübner
Am Dienstag, 10. September 2024, 17:07:57 CEST schrieb Heiko Stübner:
> Am Freitag, 6. September 2024, 03:17:42 CEST schrieb Cristian Ciocaltea:
> > The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1
> > Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a
> > Samsung IP block.
> > 
> > Add just the basic support for now, i.e. RGB output up to 4K@60Hz,
> > without audio, CEC or any of the HDMI 2.1 specific features.
> > 
> > Co-developed-by: Algea Cao 
> > Signed-off-by: Algea Cao 
> > Tested-by: Heiko Stuebner 
> > Signed-off-by: Cristian Ciocaltea 
> 
> I had switched from the v3 to this v6 in my playground-kernel today,
> with v3 I've never seen those, but now with v6 I have gotten multiple
> times:
> 
> [  805.730608] Internal error: synchronous external abort: 9610 
> [#1] PREEMPT SMP
> [  805.739764] Modules linked in: snd_soc_simple_card crct10dif_ce 
> snd_soc_simple_card_utils panthor drm_gpuvm drm_exec fuse
> [  805.752031] CPU: 3 UID: 0 PID: 775 Comm: Xorg Not tainted 
> 6.11.0-rc7-00099-g459302f1f908-dirty #934
> [  805.762143] Hardware name: Theobroma Systems RK3588-Q7 SoM on Haikou 
> devkit (DT)
> [  805.770407] pstate: 204000c9 (nzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [  805.778186] pc : regmap_mmio_read32le+0x8/0x20
> [  805.783155] lr : regmap_mmio_read+0x44/0x70
> [  805.787828] sp : 80008293b830
> [  805.791516] x29: 80008293b830 x28: 80008293bce8 x27: 
> 0001f20ab080
> [  805.799495] x26: 800081139500 x25:  x24: 
> 0010
> [  805.807472] x23:  x22: 0001f5a4b400 x21: 
> 80008293b8c4
> [  805.815450] x20: 0968 x19: 0001f5a27a80 x18: 
> 0070
> [  805.823428] x17: 000244140005 x16: 04650441043c x15: 
> 043808980804
> [  805.831406] x14: 07d8089807800780 x13: 043808980804 x12: 
> 800081133630
> [  805.839384] x11: 000244140005 x10: 04650441043c x9 : 
> 800081a59000
> [  805.847361] x8 : 07d8089807800780 x7 :  x6 : 
> 0001f5b453c0
> [  805.855339] x5 : 800080750dc0 x4 : 0968 x3 : 
> 0968
> [  805.863316] x2 : 800080751520 x1 : 0968 x0 : 
> 800083b20968
> [  805.871294] Call trace:
> [  805.874012]  regmap_mmio_read32le+0x8/0x20
> [  805.878588]  _regmap_bus_reg_read+0x6c/0xac
> [  805.883262]  _regmap_read+0x60/0xd8
> [  805.887159]  _regmap_update_bits+0xf4/0x140
> [  805.891832]  regmap_update_bits_base+0x64/0xa0
> [  805.896797]  dw_hdmi_qp_bridge_atomic_enable+0x134/0x220
> [  805.902734]  drm_atomic_bridge_chain_enable+0x54/0xc8
> [  805.908380]  drm_atomic_helper_commit_modeset_enables+0x194/0x280
> [  805.915190]  drm_atomic_helper_commit_tail_rpm+0x50/0xa0
> [  805.921125]  commit_tail+0xa0/0x1a0
> [  805.925021]  drm_atomic_helper_commit+0x17c/0x1b0
> [  805.930276]  drm_atomic_commit+0xb8/0x100
> [  805.934754]  drm_atomic_connector_commit_dpms+0xe0/0x110
> [  805.940690]  drm_mode_obj_set_property_ioctl+0x1c0/0x420
> [  805.946626]  drm_connector_property_set_ioctl+0x3c/0x68
> [  805.952465]  drm_ioctl_kernel+0xc0/0x130
> [  805.956846]  drm_ioctl+0x214/0x4a0
> [  805.960643]  __arm64_sys_ioctl+0xac/0xf8
> [  805.965025]  invoke_syscall+0x48/0x104
> [  805.969214]  el0_svc_common.constprop.0+0x40/0xe0
> [  805.974470]  do_el0_svc+0x1c/0x28
> [  805.978171]  el0_svc+0x34/0xe0
> [  805.981582]  el0t_64_sync_handler+0x120/0x12c
> [  805.986449]  el0t_64_sync+0x190/0x194
> [  805.990540] Code: d503201f d503201f f940 8b214000 (b940)
> 
> I guess that might be some clocking issue?

Forgot to add, this happens when the display has blanked and then is
supposed to unblank again.

Heiko




Re: [PATCH v6 3/3] drm/rockchip: Add basic RK3588 HDMI output support

2024-09-10 Thread Heiko Stübner
Am Freitag, 6. September 2024, 03:17:42 CEST schrieb Cristian Ciocaltea:
> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1
> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a
> Samsung IP block.
> 
> Add just the basic support for now, i.e. RGB output up to 4K@60Hz,
> without audio, CEC or any of the HDMI 2.1 specific features.
> 
> Co-developed-by: Algea Cao 
> Signed-off-by: Algea Cao 
> Tested-by: Heiko Stuebner 
> Signed-off-by: Cristian Ciocaltea 

I had switched from the v3 to this v6 in my playground-kernel today,
with v3 I've never seen those, but now with v6 I have gotten multiple
times:

[  805.730608] Internal error: synchronous external abort: 9610 
[#1] PREEMPT SMP
[  805.739764] Modules linked in: snd_soc_simple_card crct10dif_ce 
snd_soc_simple_card_utils panthor drm_gpuvm drm_exec fuse
[  805.752031] CPU: 3 UID: 0 PID: 775 Comm: Xorg Not tainted 
6.11.0-rc7-00099-g459302f1f908-dirty #934
[  805.762143] Hardware name: Theobroma Systems RK3588-Q7 SoM on Haikou devkit 
(DT)
[  805.770407] pstate: 204000c9 (nzCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[  805.778186] pc : regmap_mmio_read32le+0x8/0x20
[  805.783155] lr : regmap_mmio_read+0x44/0x70
[  805.787828] sp : 80008293b830
[  805.791516] x29: 80008293b830 x28: 80008293bce8 x27: 0001f20ab080
[  805.799495] x26: 800081139500 x25:  x24: 0010
[  805.807472] x23:  x22: 0001f5a4b400 x21: 80008293b8c4
[  805.815450] x20: 0968 x19: 0001f5a27a80 x18: 0070
[  805.823428] x17: 000244140005 x16: 04650441043c x15: 043808980804
[  805.831406] x14: 07d8089807800780 x13: 043808980804 x12: 800081133630
[  805.839384] x11: 000244140005 x10: 04650441043c x9 : 800081a59000
[  805.847361] x8 : 07d8089807800780 x7 :  x6 : 0001f5b453c0
[  805.855339] x5 : 800080750dc0 x4 : 0968 x3 : 0968
[  805.863316] x2 : 800080751520 x1 : 0968 x0 : 800083b20968
[  805.871294] Call trace:
[  805.874012]  regmap_mmio_read32le+0x8/0x20
[  805.878588]  _regmap_bus_reg_read+0x6c/0xac
[  805.883262]  _regmap_read+0x60/0xd8
[  805.887159]  _regmap_update_bits+0xf4/0x140
[  805.891832]  regmap_update_bits_base+0x64/0xa0
[  805.896797]  dw_hdmi_qp_bridge_atomic_enable+0x134/0x220
[  805.902734]  drm_atomic_bridge_chain_enable+0x54/0xc8
[  805.908380]  drm_atomic_helper_commit_modeset_enables+0x194/0x280
[  805.915190]  drm_atomic_helper_commit_tail_rpm+0x50/0xa0
[  805.921125]  commit_tail+0xa0/0x1a0
[  805.925021]  drm_atomic_helper_commit+0x17c/0x1b0
[  805.930276]  drm_atomic_commit+0xb8/0x100
[  805.934754]  drm_atomic_connector_commit_dpms+0xe0/0x110
[  805.940690]  drm_mode_obj_set_property_ioctl+0x1c0/0x420
[  805.946626]  drm_connector_property_set_ioctl+0x3c/0x68
[  805.952465]  drm_ioctl_kernel+0xc0/0x130
[  805.956846]  drm_ioctl+0x214/0x4a0
[  805.960643]  __arm64_sys_ioctl+0xac/0xf8
[  805.965025]  invoke_syscall+0x48/0x104
[  805.969214]  el0_svc_common.constprop.0+0x40/0xe0
[  805.974470]  do_el0_svc+0x1c/0x28
[  805.978171]  el0_svc+0x34/0xe0
[  805.981582]  el0t_64_sync_handler+0x120/0x12c
[  805.986449]  el0t_64_sync+0x190/0x194
[  805.990540] Code: d503201f d503201f f940 8b214000 (b940)

I guess that might be some clocking issue?




Re: [PATCH v6 3/3] drm/rockchip: Add basic RK3588 HDMI output support

2024-09-10 Thread Heiko Stübner
Am Freitag, 6. September 2024, 03:17:42 CEST schrieb Cristian Ciocaltea:
> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1
> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a
> Samsung IP block.
> 
> Add just the basic support for now, i.e. RGB output up to 4K@60Hz,
> without audio, CEC or any of the HDMI 2.1 specific features.
> 
> Co-developed-by: Algea Cao 
> Signed-off-by: Algea Cao 
> Tested-by: Heiko Stuebner 
> Signed-off-by: Cristian Ciocaltea 
> ---
>  drivers/gpu/drm/rockchip/Kconfig   |   8 +
>  drivers/gpu/drm/rockchip/Makefile  |   1 +
>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 437 
> +
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c|   2 +
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h|   1 +
>  5 files changed, 449 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig 
> b/drivers/gpu/drm/rockchip/Kconfig
> index 7df875e38517..4da7cef24f57 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -8,6 +8,7 @@ config DRM_ROCKCHIP
>   select VIDEOMODE_HELPERS
>   select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>   select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
> + select DRM_DW_HDMI_QP if ROCKCHIP_DW_HDMI_QP
>   select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
>   select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
>   select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI
> @@ -63,6 +64,13 @@ config ROCKCHIP_DW_HDMI
> enable HDMI on RK3288 or RK3399 based SoC, you should select
> this option.
>  
> +config ROCKCHIP_DW_HDMI_QP
> + bool "Rockchip specific extensions for Synopsys DW HDMI QP"

this needs a
+   select DRM_BRIDGE_CONNECTOR

now, otherwise it can't link the drm_bridge_connector_init function






Re: [PATCH v4 3/9] dt-bindings: i2c: i2c-rk3x: Add rk3576 compatible

2024-09-03 Thread Heiko Stübner
Hi Andi,

Am Dienstag, 3. September 2024, 17:46:00 CEST schrieb Andi Shyti:
> On Tue, Sep 03, 2024 at 11:22:33AM GMT, Detlev Casanova wrote:
> > Just like RK356x and RK3588, RK3576 is compatible to the existing
> > rk3399 binding.
> > 
> > Signed-off-by: Detlev Casanova 
> > Acked-by: Krzysztof Kozlowski 
> > Acked-by: Heiko Stuebner 
> 
> I will apply this after 1 and 2 have been merged.

I picked patch 2 now.

Patch 1 should go together with patch 9, but that will require the other
binding additions from this series.

So if you don't have reservations it would be cool if the i2c binding
could get merged. The i2c controller is part of the rk3576 soc and it
thankfully also is just the same as the rk356x + rk3588 soc variant
as Detlev wrote in the binding.


Thanks a lot
Heiko




Re: [PATCH v4 3/9] dt-bindings: i2c: i2c-rk3x: Add rk3576 compatible

2024-09-03 Thread Heiko Stübner
Am Dienstag, 3. September 2024, 18:47:17 CEST schrieb Andi Shyti:
> On Tue, Sep 03, 2024 at 11:59:34AM GMT, Detlev Casanova wrote:
> > On Tuesday, 3 September 2024 11:46:00 EDT Andi Shyti wrote:
> > > Hi,
> > > 
> > > On Tue, Sep 03, 2024 at 11:22:33AM GMT, Detlev Casanova wrote:
> > > > Just like RK356x and RK3588, RK3576 is compatible to the existing
> > > > rk3399 binding.
> > > > 
> > > > Signed-off-by: Detlev Casanova 
> > > > Acked-by: Krzysztof Kozlowski 
> > > > Acked-by: Heiko Stuebner 
> > > 
> > > I will apply this after 1 and 2 have been merged.
> > 
> > Sure, although it is not really dependent on 1 and 2.
> 
> yes, but I want to be sure that everything is coming in.
> 
> > > BTW, who is maintaining rockchip.yaml?
> > 
> > Heiko Stuebner is the maintainer of Rockchip SoC support.
> 
> I would guess so, but I think we should also add the entry to
> the maintainer's file :-)

now you made me doubt the wildcards we have in place ;-)

# scripts/get_maintainer.pl -f 
Documentation/devicetree/bindings/arm/rockchip.yaml
[...]
Heiko Stuebner  (maintainer:ARM/Rockchip SoC support)
[...]
linux-rockc...@lists.infradead.org (open list:ARM/Rockchip SoC support)

So Maintainers seems to be correct ... *phew* :-)




Re: [PATCH v5 1/4] dt-bindings: display: bridge: Add schema for Synopsys DW HDMI QP TX IP

2024-08-31 Thread Heiko Stübner
Hi,

Am Samstag, 31. August 2024, 08:16:26 CEST schrieb Krzysztof Kozlowski:
> On Sat, Aug 31, 2024 at 12:55:29AM +0300, Cristian Ciocaltea wrote:

> > +  clocks:
> > +minItems: 4
> > +maxItems: 6
> > +items:
> > +  - description: Peripheral/APB bus clock
> > +  - description: EARC RX biphase clock
> > +  - description: Reference clock
> > +  - description: Audio interface clock
> > +additionalItems: true
> 
> What is the usefulness of all this? How can you even be sure that each
> implementation of this core will have exactly these clocks?
>
> > +
> > +  clock-names:
> > +minItems: 4
> > +maxItems: 6
> > +items:
> > +  - const: pclk
> > +  - const: earc
> > +  - const: ref
> > +  - const: aud
> > +additionalItems: true
> > +
> > +  interrupts:
> > +minItems: 4
> > +maxItems: 5
> > +items:
> > +  - description: AVP Unit interrupt
> > +  - description: CEC interrupt
> > +  - description: eARC RX interrupt
> > +  - description: Main Unit interrupt
> 
> If these are real pins, then this seems more possible, but
> additionalItems does not make me happy.

So while not "pins", the interrupts are separately specified in the
SoC's list of interrupts in the GIC:

RK3588 has:

201  irq_hdmitx0_oavp
202  irq_hdmitx0_ocec
203  irq_hdmitx0_oearcrx
204  irq_hdmitx0_omain
392  irq_hdmitx0_hpd

and another set of all of them for hdmitx1

and RK3576 using the same hdmi IP has:

370  irq_hdmitx_oavp
371  irq_hdmitx_ocec
372  irq_hdmitx_oearcrx
373  irq_hdmitx_omain
399  irq_hdmitx_hpd

so I guess the fifth interrupt is meant to be the hotplug?
Though I guess this should be specificed in the name-list too.

>From the SoC's manual it looks like the controller is set up from
different modules.
Like AVP is the audio-video-packet-module, there is a Main and CEC Module
as well as a eARC RX controller inside. I'd guess it might be possible
other SoC vendors could leave out specific modules?


TL;DR I think those clocks and interrupts are dependent on how the
IP core was synthesized, so for now I'd think we can only guarantee
that they are true for rk3588 and rk3576.

So I guess they should move to the rockchip-specific part of the binding
until we have more hdmi-qp controllers in the field?

Heiko




Re: [PATCH v2 09/12] dt-bindings: watchdog: Add rockchip,rk3576-wdt compatible

2024-08-29 Thread Heiko Stübner
Hi Guenter,

Am Dienstag, 27. August 2024, 21:38:35 CEST schrieb Guenter Roeck:
> On 8/27/24 00:20, Heiko Stübner wrote:
> > Am Samstag, 24. August 2024, 18:44:29 CEST schrieb Guenter Roeck:
> >> On Fri, Aug 23, 2024 at 10:52:36AM -0400, Detlev Casanova wrote:
> >>> It is compatible with the other rockchip SoCs.
> >>>
> >>> Signed-off-by: Detlev Casanova 
> >>
> >> Acked-by: Guenter Roeck 
> > 
> > For my understanding, does this Ack mean you expect the patch to go in
> > with the other patches?
> > 
> Yes
> 
> > Because in theory this patch could also be picked and go through the
> > watchdog tree, similar to how the saradc binding went into the
> > iio tree already.
> > 
> 
> I thought it was all supposed to be pushed together.

I think at this point the rk3576 has still quite a number of separate
pieces. A series for clocks, one for mmc and I think there are more.

So for stuff that is obvious, like the saradc compatible Jonathan already,
picked or this watchdog compatible, a strategy of "someone picks the
individual" patch also is helpful, because it reduces the number of pieces
on the "chess board" ;-) .


So I guess both ways (wdt binding getting applied, or me just applying it
with the rest) are doable and we'll just follow what you feel comfortable
with doing.

Heiko






Re: [PATCH v2 09/12] dt-bindings: watchdog: Add rockchip,rk3576-wdt compatible

2024-08-27 Thread Heiko Stübner
Hi Guenter,

Am Samstag, 24. August 2024, 18:44:29 CEST schrieb Guenter Roeck:
> On Fri, Aug 23, 2024 at 10:52:36AM -0400, Detlev Casanova wrote:
> > It is compatible with the other rockchip SoCs.
> > 
> > Signed-off-by: Detlev Casanova 
> 
> Acked-by: Guenter Roeck 

For my understanding, does this Ack mean you expect the patch to go in
with the other patches?

Because in theory this patch could also be picked and go through the
watchdog tree, similar to how the saradc binding went into the
iio tree already.

Thanks
Heiko




Re: [PATCH v4 3/4] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller

2024-08-23 Thread Heiko Stübner
Am Donnerstag, 22. August 2024, 10:41:10 CEST schrieb Conor Dooley:
> On Thu, Aug 22, 2024 at 09:01:34AM +0200, Heiko Stübner wrote:
> > @Conor: just for me, did some shift happen in our understanding of dt-
> > best-practices in terms of syscon via phandle vs. syscon via compatible?
> > 
> > Because Rockchip boards are referencing their GRFs via phandes forever
> > but similar to the soc vs non-soc node thing, I'd like to stay on top of
> > best-practices ;-)
> 
> If IP blocks, and thus drivers, are going to be reused between devices,
> using the phandles makes sense given that it is unlikely that syscon
> nodes can make use of fallback compatibles due to bits within that "glue"
> changing between devices. It also makes sense when there are multiple
> instances of an IP on the device, which need to use different syscons.
> My goal is to ask people why they are using these type of syscons
> phandle properties, cos often they are not required at all - for example
> with clocks where you effectively need a whole new driver for every
> single soc and having a phandle property buys you nothing.

I guess I'm of two minds here.

For me at least it makes sense to spell out the dependency to the
syscon in the devicetree and not just have that hidden away inside the
driver.

But on the other hand, we already have the per-soc configuration [0]
defining which grf bits needs to be accessed, so adding a

.lanecfg1_grf_compat = "rockchip,rk3568-vo"

would not create overhad, as the grf regs and bits and rearranged
all the time anyway.


[0] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1652
taking DSI as an example, where this is even more obvious




Re: [PATCH -next 1/5] drm/rockchip: Use for_each_child_of_node_scoped()

2024-08-23 Thread Heiko Stübner
Am Freitag, 23. August 2024, 11:20:49 CEST schrieb Jinjie Ruan:
> Avoids the need for manual cleanup of_node_put() in early exits
> from the loop.
> 
> Signed-off-by: Jinjie Ruan 

Not sure if this should go in in one part or individually, but anyway
Reviewed-by: Heiko Stuebner 


>  drivers/gpu/drm/rockchip/rockchip_lvds.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c 
> b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 9a01aa450741..f5b3f18794dd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -548,7 +548,7 @@ static int rockchip_lvds_bind(struct device *dev, struct 
> device *master,
>   struct drm_encoder *encoder;
>   struct drm_connector *connector;
>   struct device_node *remote = NULL;
> - struct device_node  *port, *endpoint;
> + struct device_node  *port;
>   int ret = 0, child_count = 0;
>   const char *name;
>   u32 endpoint_id = 0;
> @@ -560,15 +560,13 @@ static int rockchip_lvds_bind(struct device *dev, 
> struct device *master,
> "can't found port point, please init lvds panel 
> port!\n");
>   return -EINVAL;
>   }
> - for_each_child_of_node(port, endpoint) {
> + for_each_child_of_node_scoped(port, endpoint) {
>   child_count++;
>   of_property_read_u32(endpoint, "reg", &endpoint_id);
>   ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
> &lvds->panel, &lvds->bridge);
> - if (!ret) {
> - of_node_put(endpoint);
> + if (!ret)
>   break;
> - }
>   }
>   if (!child_count) {
>   DRM_DEV_ERROR(dev, "lvds port does not have any children\n");
> 






Re: [PATCH v4 3/4] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller

2024-08-22 Thread Heiko Stübner
Am Donnerstag, 22. August 2024, 01:22:16 CEST schrieb Cristian Ciocaltea:
> On 8/22/24 12:38 AM, Heiko Stuebner wrote:
> > 
> > 
> > Am 21. August 2024 23:28:55 MESZ schrieb Conor Dooley :
> >> Cristian, Heiko,
> >>
> >> On Wed, Aug 21, 2024 at 11:38:01PM +0300, Cristian Ciocaltea wrote:
> >>> On 8/21/24 6:07 PM, Conor Dooley wrote:
>  On Tue, Aug 20, 2024 at 11:12:45PM +0300, Cristian Ciocaltea wrote:
> > On 8/20/24 7:14 PM, Conor Dooley wrote:
> >> On Tue, Aug 20, 2024 at 03:37:44PM +0300, Cristian Ciocaltea wrote:
> >>> On 8/19/24 7:53 PM, Conor Dooley wrote:
>  On Mon, Aug 19, 2024 at 01:29:30AM +0300, Cristian Ciocaltea wrote:
> > +  rockchip,grf:
> > +$ref: /schemas/types.yaml#/definitions/phandle
> > +description:
> > +  Most HDMI QP related data is accessed through SYS GRF regs.
> > +
> > +  rockchip,vo1-grf:
> > +$ref: /schemas/types.yaml#/definitions/phandle
> > +description:
> > +  Additional HDMI QP related data is accessed through VO1 GRF 
> > regs.
> 
>  Why are these required? What prevents you looking up the syscons by
>  compatible?
> >>>
> >>> That is for getting the proper instance:
> >>
> >> Ah, that makes sense. I am, however, curious why these have the same
> >> compatible when they have different sized regions allocated to them.
> >
> > Good question, didn't notice.  I've just checked the TRM and, in both
> > cases, the maximum register offset is within the 0x100 range.  
> > Presumably
> > this is nothing but an inconsistency, as the syscons have been added in
> > separate commits.
> 
>  Is that TRM publicly available? I do find it curious that devices sound
>  like they have different contents have the same compatible. In my view,
>  that is incorrect and they should have unique compatibles if the
>  contents (and therefore the programming model) differs.
> >>>
> >>> Don't know if there's an official location to get it from, but a quick
> >>> search on internet shows a few repos providing them, e.g. [1].
> >>>
> >>> Comparing "6.14 VO0_GRF Register Description" at pg. 777 with "6.15 
> >>> VO1_GRF
> >>> Register Description" at pg. 786 (from Part1) reveals the layout is mostly
> >>> similar, with a few variations though.
> >>
> >> Page references and everything, thank you very much. I don't think those
> >> two GRFs should have the same compatibles, they're, as you say, similar
> >> but not identical. Seems like a bug to me!
> >>
> >> Heiko, what do you think?
> > 
> > Yes, while the register names sound similar, looking at the bit
> > definitions this evening revealed that they handle vastly different
> > settings.
> > 
> > So I guess we should fix the compatibles. They are all about graphics
> > stuff and HDMI actually is the first output, so right now WE can at least
> > still claim the no-users joker ;-)
> 
> I couldn't find any driver doing a lookup for them by compatible, so I
> think it's fine to fix them - should we go for "rockchip,rk3588-vo0-grf" and
> "rockchip,rk3588-vo1-grf", respectively?

yep. While things like the MIPICDPHY{0,1}_GRF really are identifcal and
serve two separate controllers ... vo0 and vo1 are very different entities,
so fixing the compatible to reflect that makes a lot of sense.


> vo0_grf seems to be used by the usbdp phy nodes:
> 
> usbdp_phy0: phy@fed8 {
> compatible = "rockchip,rk3588-usbdp-phy";
> [...]
> rockchip,vo-grf = <&vo0_grf>;
> [...]
> 
> Same for "usbdp_phy1: phy@fed9".
> 
> While vo1_grf is present in:
> 
> vop: vop@fdd9 {
> compatible = "rockchip,rk3588-vop";
> [...]
> rockchip,vo1-grf = <&vo1_grf>;
> [...]
> 
> I guess it's too late to drop them while updating the related drivers
> accordingly, hence I wonder if we should keep using the phandles for this
> HDMI thing as well, for consistency reasons.

For the property naming, I guess it just tells the driver which "vo"-grf
to use, so the vop is more explicit in naming it vo1-grf even the vo-grf
in the usbdp phy won't hurt too much.

Of course we can still look up the grf by compatible and deprecate the
phandle references.


@Conor: just for me, did some shift happen in our understanding of dt-
best-practices in terms of syscon via phandle vs. syscon via compatible?

Because Rockchip boards are referencing their GRFs via phandes forever
but similar to the soc vs non-soc node thing, I'd like to stay on top of
best-practices ;-)


Heiko


> > Heiko
> > 
> >>
> >>> [1] https://github.com/FanX-Tek/rk3588-TRM-and-Datasheet
> >>>
> >
> >>>   vo0_grf: syscon@fd5a6000 {
> >>>   compatible = "rockchip,rk3588-vo-grf", "syscon";
> >>>   reg = <0x0 0xfd5a6000 0x0 0x2000>;
> >>>   clocks = <&cru PCLK_VO0GRF>;
> >>>   };
> >>>
> >>

Re: [PATCH v3 2/2] drm/panfrost: Add cycle counter job requirement

2024-08-20 Thread Heiko Stübner
Am Montag, 19. August 2024, 10:02:23 CEST schrieb Mary Guillemard:
> Extend the uAPI with a new job requirement flag for cycle
> counters. This requirement is used by userland to indicate that a job
> requires cycle counters or system timestamp to be propagated. (for use
> with write value timestamp jobs)
> 
> We cannot enable cycle counters unconditionally as this would result in
> an increase of GPU power consumption. As a result, they should be left
> off unless required by the application.
> 
> If a job requires cycle counters or system timestamps propagation, we
> must enable cycle counting before issuing a job and disable it right
> after the job completes.
> 
> Since this extends the uAPI and because userland needs a way to advertise
> features like VK_KHR_shader_clock conditionally, we bumps the driver
> minor version.
> 
> v2:
> - Rework commit message
> - Squash uAPI changes and implementation in this commit
> - Simplify changes based on Steven Price comments
> 
> v3:
> - Add Steven Price r-b
> - Fix a codestyle issue
> 
> Signed-off-by: Mary Guillemard 
> Reviewed-by: Steven Price 
On a rk3588-tiger with matching MESA build and 
"RUSTICL_ENABLE=panfrost clpeak"

Tested-by: Heiko Stuebner 

Without this change, clpeak fails with
clCreateCommandQueue (-35)

I guess this is mainly applicable to the timestamp part, but that is
partially in this commit too.


Heiko




Re: [PATCH v3 1/2] drm/panfrost: Add SYSTEM_TIMESTAMP and SYSTEM_TIMESTAMP_FREQUENCY parameters

2024-08-20 Thread Heiko Stübner
Am Montag, 19. August 2024, 10:02:22 CEST schrieb Mary Guillemard:
> Expose system timestamp and frequency supported by the GPU.
> 
> Mali uses an external timer as GPU system time. On ARM, this is wired to
> the generic arch timer so we wire cntfrq_el0 as device frequency.
> 
> This new uAPI will be used in Mesa to implement timestamp queries and
> VK_KHR_calibrated_timestamps.
> 
> v2:
> - Rewrote to use GPU timestamp register
> - Add missing include for arch_timer_get_cntfrq
> - Rework commit message
> 
> v3:
> - Move panfrost_cycle_counter_get and panfrost_cycle_counter_put to
>   panfrost_ioctl_query_timestamp
> - Handle possible overflow in panfrost_timestamp_read
> 
> Signed-off-by: Mary Guillemard 


On a rk3588-tiger with matching MESA build and 
"RUSTICL_ENABLE=panfrost clpeak"

Tested-by: Heiko Stuebner 

Without this change, clpeak fails with
clCreateCommandQueue (-35)




Re: [PATCH v3] drm/panthor: Add DEV_QUERY_TIMESTAMP_INFO dev query

2024-08-20 Thread Heiko Stübner
Am Montag, 19. August 2024, 13:25:08 CEST schrieb Mary Guillemard:
> Expose timestamp information supported by the GPU with a new device
> query.
> 
> Mali uses an external timer as GPU system time. On ARM, this is wired to
> the generic arch timer so we wire cntfrq_el0 as device frequency.
> 
> This new uAPI will be used in Mesa to implement timestamp queries and
> VK_KHR_calibrated_timestamps.
> 
> Since this extends the uAPI and because userland needs a way to advertise
> those features conditionally, this also bumps the driver minor version.
> 
> v2:
> - Rewrote to use GPU timestamp register
> - Added timestamp_offset to drm_panthor_timestamp_info
> - Add missing include for arch_timer_get_cntfrq
> - Rework commit message
> 
> v3:
> - Add panthor_gpu_read_64bit_counter
> - Change panthor_gpu_read_timestamp to use
>   panthor_gpu_read_64bit_counter
> 
> Signed-off-by: Mary Guillemard 

On a rk3588-tiger with matching MESA build and 
"RUSTICL_ENABLE=panfrost clpeak"

Tested-by: Heiko Stuebner 




Re: [PATCH 38/86] drm/rockchip: Run DRM default client setup

2024-08-16 Thread Heiko Stübner
Am Freitag, 16. August 2024, 14:23:04 CEST schrieb Thomas Zimmermann:
> Call drm_client_setup() to run the kernel's default client setup
> for DRM. Set fbdev_probe in struct drm_driver, so that the client
> setup can start the common fbdev client.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Sandy Huang 
> Cc: "Heiko Stübner" 
> Cc: Andy Yan 

I've looked up the whole patchseries and while I can't say overly much
about the core changes, at least for the Rockchip driver, things look
like they'll stay the same even after those changes are applied, so

Acked-by: Heiko Stuebner 

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 44d769d9234d..83ea6cc8cd21 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -17,6 +17,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -191,7 +192,7 @@ static int rockchip_drm_bind(struct device *dev)
>   if (ret)
>   goto err_kms_helper_poll_fini;
>  
> - drm_fbdev_dma_setup(drm_dev, 0);
> + drm_client_setup(drm_dev, NULL);
>  
>   return 0;
>  err_kms_helper_poll_fini:
> @@ -226,6 +227,7 @@ static const struct drm_driver rockchip_drm_driver = {
>   .driver_features= DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
>   .dumb_create= rockchip_gem_dumb_create,
>   .gem_prime_import_sg_table  = rockchip_gem_prime_import_sg_table,
> + DRM_FBDEV_DMA_DRIVER_OPS,
>   .fops   = &rockchip_drm_driver_fops,
>   .name   = DRIVER_NAME,
>   .desc   = DRIVER_DESC,
> 






Re: [PATCH RESEND] drm/rockchip: Unregister platform drivers in reverse order

2024-08-15 Thread Heiko Stübner
Am Donnerstag, 15. August 2024, 19:26:54 CEST schrieb Cristian Ciocaltea:
> On 8/15/24 5:21 PM, Heiko Stübner wrote:
> > Am Donnerstag, 8. August 2024, 13:58:02 CEST schrieb Cristian Ciocaltea:
> >> Move rockchip_drm_platform_driver unregistration after its sub-drivers,
> >> which ensures all drivers are unregistered in the reverse order used
> >> when they were registered.
> > 
> > are you sure about that?
> > 
> > I.e. currently rockchip_drm_init() does 
> >   platform_register_drivers(rockchip_sub_drivers, ...)
> > to register the sub-drivers and only after that registers the main
> > drm-platform-driver
> > 
> > rockchip_drm_fini currently does the reverse of first unregistering the
> > main drm-platform-driver and after that unregistering the array of sub-
> > drivers.
> > 
> > 
> > So as it stands right now, rockchip_drm_fini does already do exactly the
> > reverse when de-registering.
> 
> Indeed, somehow I overlooked this while debugging some module unloading
> issues.  I guess it just felt more naturally to have the subdrivers
> unregistered first.
> 
> Out of curiosity to see if there's a common pattern for handling this, I
> found that most drivers do indeed unregister the subdrivers before the main
> platform one, but weirdly enough, some of them do also keep the same order
> on registration, similarily to what this patch unintentionally does:
> 
>   drivers/power/supply/ab8500_charger.c
>   drivers/gpu/drm/vc4/vc4_drv.c
>   drivers/gpu/drm/mcde/mcde_drv.c
> 
> Not sure if those are potential mistakes, or maybe it doesn't really matter?!
> 
> Please let me know if you have a preference for it, and I'll update the
> patch accordingly, otherwise let's just ignore it altogether.

in theory it shouldn't matter, simply because the component framework
will only bind when all driver are present and unbind when the first driver
vanishes.

But I really like doing the reverse order more - so as it is now.

You also wouldn't disable clocks, before trying to deactivate some device-
function.

So deactivating stuff in the reverse order of them getting activated is most
likely less error prone.





Re: [PATCH RESEND] drm/rockchip: Unregister platform drivers in reverse order

2024-08-15 Thread Heiko Stübner
Am Donnerstag, 8. August 2024, 13:58:02 CEST schrieb Cristian Ciocaltea:
> Move rockchip_drm_platform_driver unregistration after its sub-drivers,
> which ensures all drivers are unregistered in the reverse order used
> when they were registered.

are you sure about that?

I.e. currently rockchip_drm_init() does 
  platform_register_drivers(rockchip_sub_drivers, ...)
to register the sub-drivers and only after that registers the main
drm-platform-driver

rockchip_drm_fini currently does the reverse of first unregistering the
main drm-platform-driver and after that unregistering the array of sub-
drivers.


So as it stands right now, rockchip_drm_fini does already do exactly the
reverse when de-registering.

> Fixes: 8820b68bd378 ("drm/rockchip: Refactor the component match logic.")
> Signed-off-by: Cristian Ciocaltea 
> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 44d769d9234d..ca7b07503fbe 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -528,10 +528,9 @@ static int __init rockchip_drm_init(void)
>  
>  static void __exit rockchip_drm_fini(void)
>  {
> - platform_driver_unregister(&rockchip_drm_platform_driver);
> -
>   platform_unregister_drivers(rockchip_sub_drivers,
>   num_rockchip_sub_drivers);
> + platform_driver_unregister(&rockchip_drm_platform_driver);
>  }
>  
>  module_init(rockchip_drm_init);
> 
> ---
> base-commit: 1eb586a9782cde8e5091b9de74603e0a8386b09e
> change-id: 20240702-rk-drm-fix-unreg-9f3f29996a00
> 






Re: [PATCH v3 0/5] Add initial support for the Rockchip RK3588 HDMI TX Controller

2024-08-13 Thread Heiko Stübner
Am Mittwoch, 7. August 2024, 13:07:22 CEST schrieb Cristian Ciocaltea:
> The Rockchip RK3588 SoC family integrates the Synopsys DesignWare HDMI
> 2.1 Quad-Pixel (QP) TX controller, which is a new IP block, quite
> different from those used in the previous generations of Rockchip SoCs.
> 
> The controller supports the following features, among others:
> 
> * Fixed Rate Link (FRL)
> * Display Stream Compression (DSC)
> * 4K@120Hz and 8K@60Hz video modes
> * Variable Refresh Rate (VRR) including Quick Media Switching (QMS)
> * Fast Vactive (FVA)
> * SCDC I2C DDC access
> * Multi-stream audio
> * Enhanced Audio Return Channel (EARC)
> 
> This is the last component that needs to be supported in order to enable
> the HDMI output functionality on the RK3588 based SBCs, such as the
> RADXA Rock 5B.  The other components are the Video Output Processor
> (VOP2) and the Samsung IP based HDMI/eDP TX Combo PHY, for which basic
> support has been already made available via [1] and [2], respectively.
> 
> Please note this is a reworked version of the original series, which
> relied on a commonized dw-hdmi approach.  Since the general consensus
> was to handle it as an entirely new IP, I dropped all patches related to
> the old dw-hdmi and Rockchip glue code - a few of them might still make
> sense as general improvements and will be submitted separately.
> 
> It's worth mentioning the HDMI output support is currently limited to
> RGB output up to 4K@60Hz, without audio, CEC or any of the HDMI 2.1
> specific features.  Moreover, the VOP2 driver is not able to properly
> handle all display modes supported by the connected screens, e.g. it
> doesn't cope with non-integer refresh rates.
> 
> A possible workaround consists of enabling the display controller to
> make use of the clock provided by the HDMI PHY PLL.  This is still work
> in progress and will be submitted later, as well as the required DTS
> updates.
> 
> To facilitate testing and experimentation, all HDMI output related
> patches, including those part of this series, are available at [3].
> 
> So far I could only verify this on the RADXA Rock 5B board.

On a rk3588-tiger-haikou (including its DSI hat and my preliminary DSI
driver) it also works.

Even with both DSI and HDMI at the same time. Both hdmi plugged in on
boot and also plugging it in during runtime of the board, generates a
clean image on my 1080p display.

So, series
Tested-by: Heiko Stuebner 





Re: [PATCH v2] drm/panthor: Add DEV_QUERY_TIMESTAMP_INFO dev query

2024-08-13 Thread Heiko Stübner
Hi,

Am Montag, 12. August 2024, 14:28:15 CEST schrieb Mary Guillemard:
> Expose timestamp information supported by the GPU with a new device
> query.
> 
> Mali uses an external timer as GPU system time. On ARM, this is wired to
> the generic arch timer so we wire cntfrq_el0 as device frequency.
> 
> This new uAPI will be used in Mesa to implement timestamp queries and
> VK_KHR_calibrated_timestamps.
> 
> Since this extends the uAPI and because userland needs a way to advertise
> those features conditionally, this also bumps the driver minor version.
> 
> v2:
> - Rewrote to use GPU timestamp register
> - Added timestamp_offset to drm_panthor_timestamp_info
> - Add missing include for arch_timer_get_cntfrq
> - Rework commit message
> 
> Signed-off-by: Mary Guillemard 

I can't say much to the change itself, though did not see anything
obviously wrong with it. But I gave it a spin on one of my rk3588 boards
and together with the mesa change, it makes clpeak much more happy.

Where it originally failed to start at all due to not being able to create
its command-qeue and I had to resort to doing

cl::CommandQueue queue = cl::CommandQueue(ctx, devices[d], 
0/*CL_QUEUE_PROFILING_ENABLE*/);

the added profiling support from the timestamping is much appreciated.

Tested-by: Heiko Stuebner 


Thanks a lot
Heiko




Re: [PATCH v11 09/24] drm/rockchip: dw_hdmi: add regulator support

2024-07-04 Thread Heiko Stübner
Am Donnerstag, 4. Juli 2024, 11:09:00 CEST schrieb Diederik de Haas:
> On Friday, 22 April 2022 09:28:26 CEST Sascha Hauer wrote:
> > The RK3568 has HDMI_TX_AVDD0V9 and HDMI_TX_AVDD_1V8 supply inputs needed
> > for the HDMI port. add support for these to the driver for boards which
> > have them supplied by switchable regulators.
> > 
> > Signed-off-by: Sascha Hauer 
> > Reviewed-by: Dmitry Osipenko 
> > ---
> >  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 41 +++--
> >  1 file changed, 38 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index
> > b64cc62c7b5af..fe4f9556239ac 100644
> > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > @@ -9,6 +9,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > 
> >  #include 
> >  #include 
> > @@ -76,6 +77,8 @@ struct rockchip_hdmi {
> > struct clk *ref_clk;
> > struct clk *grf_clk;
> > struct dw_hdmi *hdmi;
> > +   struct regulator *avdd_0v9;
> > +   struct regulator *avdd_1v8;
> > struct phy *phy;
> >  };
> > 
> > @@ -226,6 +229,14 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi
> > *hdmi) return PTR_ERR(hdmi->grf_clk);
> > }
> > 
> > +   hdmi->avdd_0v9 = devm_regulator_get(hdmi->dev, "avdd-0v9");
> > +   if (IS_ERR(hdmi->avdd_0v9))
> > +   return PTR_ERR(hdmi->avdd_0v9);
> > +
> > +   hdmi->avdd_1v8 = devm_regulator_get(hdmi->dev, "avdd-1v8");
> > +   if (IS_ERR(hdmi->avdd_1v8))
> > +   return PTR_ERR(hdmi->avdd_1v8);
> > +
> > return 0;
> >  }
> > 
> > @@ -566,11 +577,23 @@ static int dw_hdmi_rockchip_bind(struct device *dev,
> > struct device *master, return ret;
> > }
> > 
> > +   ret = regulator_enable(hdmi->avdd_0v9);
> > +   if (ret) {
> > +   DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd0v9: 
> %d\n", ret);
> > +   goto err_avdd_0v9;
> > +   }
> > +
> > +   ret = regulator_enable(hdmi->avdd_1v8);
> > +   if (ret) {
> > +   DRM_DEV_ERROR(hdmi->dev, "failed to enable avdd1v8: 
> %d\n", ret);
> > +   goto err_avdd_1v8;
> > +   }
> > +
> > ret = clk_prepare_enable(hdmi->ref_clk);
> > if (ret) {
> > DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI 
> reference clock: %d\n",
> >   ret);
> > -   return ret;
> > +   goto err_clk;
> > }
> > 
> > if (hdmi->chip_data == &rk3568_chip_data) {
> > @@ -594,10 +617,19 @@ static int dw_hdmi_rockchip_bind(struct device *dev,
> > struct device *master, */
> > if (IS_ERR(hdmi->hdmi)) {
> > ret = PTR_ERR(hdmi->hdmi);
> > -   drm_encoder_cleanup(encoder);
> > -   clk_disable_unprepare(hdmi->ref_clk);
> > +   goto err_bind;
> > }
> > 
> > +   return 0;
> > +
> > +err_bind:
> > +   clk_disable_unprepare(hdmi->ref_clk);
> > +   drm_encoder_cleanup(encoder);
> > +err_clk:
> > +   regulator_disable(hdmi->avdd_1v8);
> > +err_avdd_1v8:
> > +   regulator_disable(hdmi->avdd_0v9);
> > +err_avdd_0v9:
> > return ret;
> >  }
> > 
> > @@ -608,6 +640,9 @@ static void dw_hdmi_rockchip_unbind(struct device *dev,
> > struct device *master,
> > 
> > dw_hdmi_unbind(hdmi->hdmi);
> > clk_disable_unprepare(hdmi->ref_clk);
> > +
> > +   regulator_disable(hdmi->avdd_1v8);
> > +   regulator_disable(hdmi->avdd_0v9);
> >  }
> > 
> >  static const struct component_ops dw_hdmi_rockchip_ops = {
> 
> Is it possible to probe for those avdd_0v9 and avdd_1v8 regulators only on 
> devices that should have them?
> 
> On a Rock64 (rk3328), but probably for all VOP1 devices, they're not present 
> and that results in the following warnings:
> dwhdmi-rockchip ff3c.hdmi: supply avdd-0v9 not found, using dummy 
> regulator
> dwhdmi-rockchip ff3c.hdmi: supply avdd-1v8 not found, using dummy 
> regulator

counter-argument, why not define them in the dts?
I.e. looking at the rock64 schematics, you want the dvideo_avdd_1v8 (from
LDO1) and dvideo_avdd_1v0 (from LDO3) if I'm not mistaken. Why this stuff
is called dvideo and not hdmi in there I have no clue ;-)


Heiko




Re: [PATCH 02/13] clk: rockchip: Set parent rate for DCLK_VOP clock on RK3228

2024-06-17 Thread Heiko Stübner
Am Samstag, 15. Juni 2024, 19:03:53 CEST schrieb Jonas Karlman:
> Similar to DCLK_LCDC on RK3328, the DCLK_VOP on RK3228 is typically
> parented by the hdmiphy clk and it is expected that the DCLK_VOP and
> hdmiphy clk rate are kept in sync.
> 
> Use CLK_SET_RATE_PARENT and CLK_SET_RATE_NO_REPARENT flags, same as used
> on RK3328, to make full use of all possible supported display modes.
> 
> Fixes: 0a9d4ac08ebc ("clk: rockchip: set the clock ids for RK3228 VOP")
> Fixes: 307a2e9ac524 ("clk: rockchip: add clock controller for rk3228")
> Signed-off-by: Jonas Karlman 

did your mailer have a hickup? Somehow I got patch2 (only this one)
2 times




Re: [PATCH 00/14] improve Analogix DP AUX channel handling

2024-06-10 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 17:11:15 CEST schrieb Lucas Stach:
> Currently the AUX channel support in the Analogix DP driver is severely
> limited as the AUX block of the bridge is only initialized when the video
> link is to be enabled. This is okay for the purposes of link training,
> but does not allow to detect displays by probing for EDID. This series
> reworks the driver to allow AUX transactions before the video link is
> active.
> 
> As this requires to rework some of the controller initialization and
> also handling of both internal and external clocks, the series includes
> quite a few changes to add better runtime PM handling.
> 
> Lucas Stach (14):
>   drm/bridge: analogix_dp: remove unused platform power_on_end callback
>   drm/rockchip: analogix_dp: add runtime PM handling
>   drm/bridge: analogix_dp: register AUX bus after enabling runtime PM
>   drm/bridge: analogix_dp: handle clock via runtime PM
>   drm/bridge: analogix_dp: remove unused analogix_dp_remove
>   drm/bridge: analogix_dp: remove clk handling from
> analogix_dp_set_bridge
>   drm/bridge: analogix_dp: move platform and PHY power handling into
> runtime PM
>   drm/bridge: analogix_dp: move basic controller init into runtime PM
>   drm/bridge: analogix_dp: remove PLL lock check from
> analogix_dp_config_video
>   drm/bridge: analogix_dp: move macro reset after link bandwidth setting
>   drm/bridge: analogix_dp: don't wait for PLL lock too early
>   drm/bridge: analogix_dp: simplify and correct PLL lock checks
>   drm/bridge: analogix_dp: only read AUX status when an error occured
>   drm/bridge: analogix_dp: handle AUX transfer timeouts

my knowledge of Analgix-dp internals is limited, but at least both
rk3288-veyron and rk3399 gru still had working displays with this series
applied and both device classes using the analogix-dp for their main
display.

So on rk3288-veyron and rk3399-gru
Tested-by: Heiko Stuebner 




Re: [PATCH 02/14] drm/rockchip: analogix_dp: add runtime PM handling

2024-06-10 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 17:11:17 CEST schrieb Lucas Stach:
> Hook up the runtime PM suspend/resume paths to make the rockchip
> glue behave more like the exynos one. The same suspend/resume
> functions are used for system sleep via the runtime PM force
> suspend/resume.
> 
> Signed-off-by: Lucas Stach 

Reviewed-by: Heiko Stuebner 

A rk3399-gru Chromebook was able to suspend and wake up again
with working display both before and after.


Heiko




Re: [PATCH 13/14] drm/bridge: synopsys: Add DW HDMI QP TX controller driver

2024-06-06 Thread Heiko Stübner
Am Donnerstag, 6. Juni 2024, 11:53:23 CEST schrieb Cristian Ciocaltea:
> On 6/5/24 5:48 PM, Heiko Stübner wrote:
> > Am Samstag, 1. Juni 2024, 15:12:35 CEST schrieb Cristian Ciocaltea:
> >> The Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP) TX controller supports
> >> the following features, among others:
> >>
> >> * Fixed Rate Link (FRL)
> >> * 4K@120Hz and 8K@60Hz video modes
> >> * Variable Refresh Rate (VRR) including Quick Media Switching (QMS), aka
> >>   Cinema VRR
> >> * Fast Vactive (FVA), aka Quick Frame Transport (QFT)
> >> * SCDC I2C DDC access
> >> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4
> >> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds
> >> * Multi-stream audio
> >> * Enhanced Audio Return Channel (EARC)
> >>
> >> Add driver to enable basic support, i.e. RGB output up to 4K@60Hz,
> >> without audio, CEC or any HDMI 2.1 specific features.
> >>
> >> Co-developed-by: Algea Cao 
> >> Signed-off-by: Algea Cao 
> >> Signed-off-by: Cristian Ciocaltea 
> >> ---
> >>  drivers/gpu/drm/bridge/synopsys/Makefile |   2 +-
> >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 
> >> +
> >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 
> >> +++
> >>  include/drm/bridge/dw_hdmi.h |   8 +
> >>  4 files changed, 1627 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/synopsys/Makefile 
> >> b/drivers/gpu/drm/bridge/synopsys/Makefile
> >> index ce715562e9e5..8354e4879f70 100644
> >> --- a/drivers/gpu/drm/bridge/synopsys/Makefile
> >> +++ b/drivers/gpu/drm/bridge/synopsys/Makefile
> > 
> >> +static int dw_hdmi_qp_i2c_read(struct dw_hdmi *hdmi,
> >> + unsigned char *buf, unsigned int length)
> >> +{
> >> +  struct dw_hdmi_i2c *i2c = hdmi->i2c;
> >> +  int stat;
> >> +
> >> +  if (!i2c->is_regaddr) {
> >> +  dev_dbg(hdmi->dev, "set read register address to 0\n");
> >> +  i2c->slave_reg = 0x00;
> >> +  i2c->is_regaddr = true;
> >> +  }
> >> +
> >> +  while (length--) {
> >> +  reinit_completion(&i2c->cmp);
> >> +
> >> +  dw_hdmi_qp_mod(hdmi, i2c->slave_reg++ << 12, I2CM_ADDR,
> >> + I2CM_INTERFACE_CONTROL0);
> >> +
> >> +  dw_hdmi_qp_mod(hdmi, I2CM_FM_READ, I2CM_WR_MASK,
> >> + I2CM_INTERFACE_CONTROL0);
> > 
> > Somehow the segment handling is present in the rest of the i2c code here, 
> > but
> > not the actual handling for reads.
> > 
> > The vendor-kernel does:
> > 
> > -   dw_hdmi_qp_mod(hdmi, I2CM_FM_READ, I2CM_WR_MASK,
> > -  I2CM_INTERFACE_CONTROL0);
> > +   if (i2c->is_segment)
> > +   dw_hdmi_qp_mod(hdmi, I2CM_EXT_READ, I2CM_WR_MASK,
> > +  I2CM_INTERFACE_CONTROL0);
> > +   else
> > +   dw_hdmi_qp_mod(hdmi, I2CM_FM_READ, I2CM_WR_MASK,
> > +  I2CM_INTERFACE_CONTROL0);
> 
> Hmm, for some reason this is not present in the stable-5.10-rock5 branch 
> I've been using as an implementation reference:
> 
> https://github.com/radxa/kernel/blob/stable-5.10-rock5/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c#L760
> 
> Is there an updated fork?

I think the radxa code-base is quite old in terms of sdk-version it's based on.
Grabbing a 6.1 branch from Radxa shows it in:
https://github.com/radxa/kernel/blob/linux-6.1-stan-rkr1/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c#L995

> > Without this change, connecting to a DVI display does not work, and
> > reading the EDID ends in the "i2c read error" below.
> > 
> > Adding the segment handling as above makes the DVI connection
> > work (as it does in the vendor-kernel).
> > 
> > So it would be nice if you could maybe incorporate this in the next version?
> 
> Sure, thanks for pointing this out!
> 
> Cristian
> 






Re: [PATCH 13/14] drm/bridge: synopsys: Add DW HDMI QP TX controller driver

2024-06-05 Thread Heiko Stübner
Am Mittwoch, 5. Juni 2024, 21:58:23 CEST schrieb Luis de Arquer:
> On 6/5/24 16:48, Heiko Stübner wrote:
> > Without this change, connecting to a DVI display does not work, and
> > reading the EDID ends in the "i2c read error" below.
> 
> I had a lot of problems initially with the vendor driver on my DVI 
> display, and am aware that several changes were required.
> 
> However, I tested Cristian patch and worked fine. All modes were 
> apparently detected from the display and they all worked. But maybe I 
> was just lucky and it was using a somehow cached table, I can't say.
> 
> This is an AOC DVI display from 2011 with a passive adapter.

For me it is an LG IPS235. On its native hdmi input I always get regular
1080p output. But on its DVI input I always ran into the i2c read error
before adding that change.

I guess I should determine which reads actually fail.




Re: [PATCH 13/14] drm/bridge: synopsys: Add DW HDMI QP TX controller driver

2024-06-05 Thread Heiko Stübner
Am Samstag, 1. Juni 2024, 15:12:35 CEST schrieb Cristian Ciocaltea:
> The Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP) TX controller supports
> the following features, among others:
> 
> * Fixed Rate Link (FRL)
> * 4K@120Hz and 8K@60Hz video modes
> * Variable Refresh Rate (VRR) including Quick Media Switching (QMS), aka
>   Cinema VRR
> * Fast Vactive (FVA), aka Quick Frame Transport (QFT)
> * SCDC I2C DDC access
> * TMDS Scrambler enabling 2160p@60Hz with RGB/YCbCr4:4:4
> * YCbCr4:2:0 enabling 2160p@60Hz at lower HDMI link speeds
> * Multi-stream audio
> * Enhanced Audio Return Channel (EARC)
> 
> Add driver to enable basic support, i.e. RGB output up to 4K@60Hz,
> without audio, CEC or any HDMI 2.1 specific features.
> 
> Co-developed-by: Algea Cao 
> Signed-off-by: Algea Cao 
> Signed-off-by: Cristian Ciocaltea 
> ---
>  drivers/gpu/drm/bridge/synopsys/Makefile |   2 +-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 787 +
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.h | 831 
> +++
>  include/drm/bridge/dw_hdmi.h |   8 +
>  4 files changed, 1627 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/Makefile 
> b/drivers/gpu/drm/bridge/synopsys/Makefile
> index ce715562e9e5..8354e4879f70 100644
> --- a/drivers/gpu/drm/bridge/synopsys/Makefile
> +++ b/drivers/gpu/drm/bridge/synopsys/Makefile

> +static int dw_hdmi_qp_i2c_read(struct dw_hdmi *hdmi,
> +unsigned char *buf, unsigned int length)
> +{
> + struct dw_hdmi_i2c *i2c = hdmi->i2c;
> + int stat;
> +
> + if (!i2c->is_regaddr) {
> + dev_dbg(hdmi->dev, "set read register address to 0\n");
> + i2c->slave_reg = 0x00;
> + i2c->is_regaddr = true;
> + }
> +
> + while (length--) {
> + reinit_completion(&i2c->cmp);
> +
> + dw_hdmi_qp_mod(hdmi, i2c->slave_reg++ << 12, I2CM_ADDR,
> +I2CM_INTERFACE_CONTROL0);
> +
> + dw_hdmi_qp_mod(hdmi, I2CM_FM_READ, I2CM_WR_MASK,
> +I2CM_INTERFACE_CONTROL0);

Somehow the segment handling is present in the rest of the i2c code here, but
not the actual handling for reads.

The vendor-kernel does:

-   dw_hdmi_qp_mod(hdmi, I2CM_FM_READ, I2CM_WR_MASK,
-  I2CM_INTERFACE_CONTROL0);
+   if (i2c->is_segment)
+   dw_hdmi_qp_mod(hdmi, I2CM_EXT_READ, I2CM_WR_MASK,
+  I2CM_INTERFACE_CONTROL0);
+   else
+   dw_hdmi_qp_mod(hdmi, I2CM_FM_READ, I2CM_WR_MASK,
+  I2CM_INTERFACE_CONTROL0);

Without this change, connecting to a DVI display does not work, and
reading the EDID ends in the "i2c read error" below.

Adding the segment handling as above makes the DVI connection
work (as it does in the vendor-kernel).

So it would be nice if you could maybe incorporate this in the next version?


Thanks
Heiko


> +
> + stat = wait_for_completion_timeout(&i2c->cmp, HZ / 10);
> + if (!stat) {
> + dev_err(hdmi->dev, "i2c read timed out\n");
> + dw_hdmi_qp_write(hdmi, 0x01, I2CM_CONTROL0);
> + return -EAGAIN;
> + }
> +
> + /* Check for error condition on the bus */
> + if (i2c->stat & I2CM_NACK_RCVD_IRQ) {
> + dev_err(hdmi->dev, "i2c read error\n");
> + dw_hdmi_qp_write(hdmi, 0x01, I2CM_CONTROL0);
> + return -EIO;
> + }
> +
> + *buf++ = dw_hdmi_qp_read(hdmi, I2CM_INTERFACE_RDDATA_0_3) & 
> 0xff;
> + dw_hdmi_qp_mod(hdmi, 0, I2CM_WR_MASK, I2CM_INTERFACE_CONTROL0);
> + }
> +
> + i2c->is_segment = false;
> +
> + return 0;
> +}





Re: [PATCH v1 1/1] treewide: Align match_string() with sysfs_match_string()

2024-06-04 Thread Heiko Stübner
Am Sonntag, 2. Juni 2024, 17:57:12 CEST schrieb Andy Shevchenko:
> Make two APIs look similar. Hence convert match_string() to be
> a 2-argument macro. In order to avoid unneeded churn, convert
> all users as well. There is no functional change intended.
> 
> Signed-off-by: Andy Shevchenko 
> ---
> 
> Compile tested with `make allyesconfig` and `make allmodconfig`
> on x86_64, arm, aarch64, powerpc64 (8 builds total).
> 
> I guess the best is to apply it to Linus' tree directly.
> And now it seems a good timing as there are no new users
> of this API either in v6.10-rcX, or in Linux Next.
> 
> But if you think differently, tell me.
> 

For the Rockchip clock part

>  drivers/clk/rockchip/clk.c|  4 +--

[...]

> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 73d2cbdc716b..30414d081f46 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -266,8 +266,8 @@ static struct clk *rockchip_clk_register_frac_branch(
>   struct clk *mux_clk;
>   int ret;
>  
> - frac->mux_frac_idx = match_string(child->parent_names,
> -   child->num_parents, name);
> + frac->mux_frac_idx = __match_string(child->parent_names,
> + child->num_parents, name);
>   frac->mux_ops = &clk_mux_ops;
>   frac->clk_nb.notifier_call = rockchip_clk_frac_notifier_cb;
>  

Acked-by: Heiko Stuebner 




Re: [PATCH 0/1] Fix the port mux of VP2

2024-05-29 Thread Heiko Stübner
Am Mittwoch, 29. Mai 2024, 17:55:25 CEST schrieb Diederik de Haas:
> On Thursday, 25 April 2024 17:19:58 CEST Heiko Stuebner wrote:
> > On Mon, 22 Apr 2024 18:19:04 +0800, Andy Yan wrote:
> > > From: Andy Yan 
> > > 
> > > The port mux bits of VP2 should be defined by
> > > RK3568_OVL_PORT_SET__PORT2_MUX, this maybe a copy and paste error when
> > > this driver first introduced.> 
> > > Hi Heiko:
> > >Maybe thi is the problem you met when you porting the dsi2 driver.
> > > 
> > 
> > Applied, thanks!
> > 
> > [1/1] drm/rockchip: vop2: Fix the port mux of VP2
> >   commit: 2bdb481bf7a93c22b9fea8daefa2834aab23a70f
> 
> Wasn't this patch supposed to be part of 6.10-rc1?

Looking at the drm-misc tree, the last tag for the drm-misc to drm-main
merge is labeled drm-misc-next-2024-04-25, same day as I applied the
patch.

In theory I think -rc6 is the cutoff for drm-misc changes for mainline,
which would've been the 28th of april, but there might've been simple
hickups preventing that last merge, resulting in the patch missing an
early cutoff.


On the other hand, somehow Torvald's tree actually has this commit [0],
just with a "Notice: this object is not reachable from any branch."
Possibly some drm-merge-mayhem?

All very confusing.

@Thomas, @Marten: do you possible have an idea what might've happened?


Heiko




[0] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2bdb481bf7a93c22b9fea8daefa2834aab23a70f




Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066

2024-05-27 Thread Heiko Stübner
Hey,

Am Dienstag, 28. Mai 2024, 00:13:59 CEST schrieb Val Packett:
> On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner 
>  wrote:
> > Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> >>  On the RK3066, there is a bit that must be cleared, otherwise
> >>  the picture does not show up
> >> on the display (at least for RGB).
> >> 
> >>  Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> >>  Cc: sta...@vger.kernel.org
> >>  Signed-off-by: Val Packett 
> >>  ---
> >>  v2: doing this on vblank makes more sense; added fixes tag
> > 
> > can you give a rationale for this please?
> > 
> > I.e. does this dma-stop bit need to be set on each vblank that happens
> > to push this frame to the display somehow?
> 
> 
> The only things I'm 100% sure about:
> 
> - that bit is called dma_stop in the Android kernel's header;
> - without ever setting that bit to 1, it was getting set to 1 by the 
> chip itself, as logging the register on flush was showing a 1 in that 
> position (it was the only set bit - I guess others aren't readable 
> after cfg_done?);
> - without clearing it "between" frames, the whole screen is always 
> filled with noise, the picture is not visible.
> 
> The rest is at least a bit (ha) speculative:
> 
> As I understand from what the name implies, the hardware sets it to 
> indicate that it has scanned out the frame and is waiting for 
> acknowledgment (clearing) to be able to scan out the next frame. I 
> guess it's a redundant synchronization mechanism that was removed in 
> later iterations of the VOP hardware block.
> 
> I've been trying to see if moving where I clear the bit affects the 
> sort-of-tearing-but-vertical glitches that sometimes happen, especially 
> early on after the system has just booted, but that seems to be 
> completely unrelated pixel clock craziness (the Android kernel runs the 
> screen at 66 fps, interestingly).
> 
> I'm fairly confident that both places are "correct". The reason I'm 
> more on the side of vblank now is that it made logical sense to me when 
> I thought about it more: acknowledging that the frame has been scanned 
> out is a reaction to the frame having been scanned out. It's a 
> consequence of *that* that the acknowledgment is required for the next 
> frame to be drawn.
> 
> Unless we can get the opinion of someone closely familiar with this 
> decade-old hardware, we only have this reasoning to go off of :)

Actually that reasoning was exactly what I was hoping for :-) .
And it actually also makes perfect sense.

I was somehow thinking this needs to be set only when starting output
and not as sort of an Ack.

Could you do a v3 with:
- the findings from above slightly condensed in the commit message
  It's really helpful when someone stumbles onto that commit 10 years
  from now and can get this really helpful explanation from the commit
  message.
- sending it as a _new_ thread
  Having v2 as reply to v1 patches confuses tooling that then can't
  distinguish what is actually part of this v2


Thanks a lot
Heiko




Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066

2024-05-27 Thread Heiko Stübner
Hi Val,

Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> On the RK3066, there is a bit that must be cleared, otherwise
> the picture does not show up on the display (at least for RGB).
> 
> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Val Packett 
> ---
> v2: doing this on vblank makes more sense; added fixes tag

can you give a rationale for this please?

I.e. does this dma-stop bit need to be set on each vblank that happens
to push this frame to the display somehow?

Because at least in theory atomic_flush where this was living in in v1,
might happen at a slower interval?


Thanks
Heiko

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 ++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 +
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 +
>  3 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index a13473b2d..2731fe2b2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1766,6 +1766,12 @@ static void vop_handle_vblank(struct vop *vop)
>   }
>   spin_unlock(&drm->event_lock);
>  
> + if (VOP_HAS_REG(vop, common, dma_stop)) {
> + spin_lock(&vop->reg_lock);
> + VOP_REG_SET(vop, common, dma_stop, 0);
> + spin_unlock(&vop->reg_lock);
> + }
> +
>   if (test_and_clear_bit(VOP_PENDING_FB_UNREF, &vop->pending))
>   drm_flip_work_commit(&vop->fb_unref_work, system_unbound_wq);
>  }
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index b33e5bdc2..0cf512cc1 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -122,6 +122,7 @@ struct vop_common {
>   struct vop_reg lut_buffer_index;
>   struct vop_reg gate_en;
>   struct vop_reg mmu_en;
> + struct vop_reg dma_stop;
>   struct vop_reg out_mode;
>   struct vop_reg standby;
>  };
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c 
> b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index b9ee02061..9bcb40a64 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -466,6 +466,7 @@ static const struct vop_output rk3066_output = {
>  };
>  
>  static const struct vop_common rk3066_common = {
> + .dma_stop = VOP_REG(RK3066_SYS_CTRL0, 0x1, 0),
>   .standby = VOP_REG(RK3066_SYS_CTRL0, 0x1, 1),
>   .out_mode = VOP_REG(RK3066_DSP_CTRL0, 0xf, 0),
>   .cfg_done = VOP_REG(RK3066_REG_CFG_DONE, 0x1, 0),
> 






Re: [PATCH v4 00/10] drm/verisilicon : support DC8200 and inno hdmi

2024-05-21 Thread Heiko Stübner
Hi Alex,

Am Dienstag, 21. Mai 2024, 12:58:07 CEST schrieb keith:
> Verisilicon/DC8200 display controller IP has 2 display pipes and each 
> pipe support a primary plane and a cursor plane . 
> In addition, there are four overlay planes as two display pipes common 
> resources.
> 
> The first display pipe is bound to the inno HDMI encoder.
> The second display pipe is bound to a simple encoder, which is used to
> find dsi bridge by dts node. 
> 
> Patch 1 adds YAML schema for JH7110 display pipeline.
> 
> Patches 2 to 3 add inno common api and match the ROCKCHIP inno hdmi driver 
> by calling the common api. 
> The collating public interface is based on ROCKCHIP inno hdmi, 
> and it can be resused by JH7110 inno hdmi.
> Those common api are tested on rk-3128 SDK, which kernel version is 4.x. 

as you were working on the rk3128-inno-hdmi variant recently
and I don't really have a rk3036 or rk3128 in working condition
right now, could you give this series a try.

For reference, the full series is at lore:
https://lore.kernel.org/dri-devel/20240521105817.3301-1-keith.z...@starfivetech.com/

and generalizes the inno-hdmi driver into the bridge model we
have in a number of other places already.


Thanks
Heiko



> step1, make sure the process is consistent with the latest kernel version.
> step2, just remove the interface and add a common interface. 
> 
> Patches 4 to 8 add kms driver for dc8200 display controller.
> 
> Patch 9 adds inno hdmi support for JH7110 display pipeline.
> 
> Patch 10 adds a simple encoder.
> 
> This patchset should be applied on next branch.
> 
> V1:
> Changes since v1:
> - Further standardize the yaml file.
> - Dts naming convention improved.
> - Fix the problem of compiling and loading ko files.
> - Use drm new api to automatically manage resources.
> - Drop vs_crtc_funcs&vs_plane_funcs, subdivide the plane's help interface.
> - Reduce the modifiers unused.
> - Optimize the hdmi driver code
> 
> V2:
> Changes since v2:
> - fix the error about checking the yaml file.
> - match drm driver GEM DMA API.
> - Delete the custom crtc property .
> - hdmi use drmm_ new api to automatically manage resources.
> - update the modifiers comments.
> - enabling KASAN, fix the error during removing module 
> 
> V3:
> Changes since v3:
> - Delete the custom plane property.
> - Delete the custom fourcc modifiers.
> - Adjust the calculation mode of hdmi pixclock.
> - Add match data for dc8200 driver.
> - Adjust some magic values.
> - Add a simple encoder for dsi output.
> 
> V4:
> Changes since v4:
> - Delete the display subsystem module as all crtcs and planes are a driver.
> - Delete the custom struct, directly use the drm struct data.
> - Tidy up the inno hdmi public interface.
> - Add a simple encoder for dsi output.
> 
> keith (10):
>   dt-bindings: display: Add YAML schema for JH7110 display pipeline
>   drm/bridge: add common api for inno hdmi
>   drm/rockchip:hdmi: migrate to use inno-hdmi bridge driver
>   drm/vs: Add hardware funcs for vs.
>   drm/vs: add vs mode config init
>   drm/vs: add vs plane api
>   drm/vs: add ctrc fun
>   drm/vs: add vs drm master driver
>   drm/vs: Innosilicon HDMI support
>   drm/vs: add simple dsi encoder
> 
>  .../display/bridge/innosilicon,inno-hdmi.yaml |   49 +
>  .../display/rockchip/rockchip,inno-hdmi.yaml  |   27 +-
>  .../starfive/starfive,dsi-encoder.yaml|   92 ++
>  .../starfive/starfive,jh7110-dc8200.yaml  |  169 +++
>  .../starfive/starfive,jh7110-inno-hdmi.yaml   |   75 ++
>  .../soc/starfive/starfive,jh7110-syscon.yaml  |1 +
>  MAINTAINERS   |   11 +
>  drivers/gpu/drm/Kconfig   |2 +
>  drivers/gpu/drm/Makefile  |1 +
>  drivers/gpu/drm/bridge/Kconfig|2 +
>  drivers/gpu/drm/bridge/Makefile   |1 +
>  drivers/gpu/drm/bridge/innosilicon/Kconfig|6 +
>  drivers/gpu/drm/bridge/innosilicon/Makefile   |2 +
>  .../gpu/drm/bridge/innosilicon/inno-hdmi.c|  587 +
>  .../gpu/drm/bridge/innosilicon/inno-hdmi.h|   97 ++
>  drivers/gpu/drm/rockchip/Kconfig  |1 +
>  drivers/gpu/drm/rockchip/Makefile |2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi-rockchip.c |  517 
>  .../{inno_hdmi.h => inno_hdmi-rockchip.h} |   45 -
>  drivers/gpu/drm/rockchip/inno_hdmi.c  | 1073 -
>  drivers/gpu/drm/verisilicon/Kconfig   |   23 +
>  drivers/gpu/drm/verisilicon/Makefile  |   11 +
>  .../gpu/drm/verisilicon/inno_hdmi-starfive.c  |  481 
>  .../gpu/drm/verisilicon/inno_hdmi-starfive.h  |  152 +++
>  drivers/gpu/drm/verisilicon/vs_crtc.c |  241 
>  drivers/gpu/drm/verisilicon/vs_crtc.h |   17 +
>  drivers/gpu/drm/verisilicon/vs_dc_hw.c| 1060 
>  drivers/gpu/drm/verisilicon/vs_dc_hw.h|  493 
>  drivers/gpu/drm/verisilicon/vs_drv.c  |  721 +++
>  drivers/gpu/drm/verisili

Re: [PATCH 3/3] dt-bindings: display: vop2: Add VP clock resets

2024-05-15 Thread Heiko Stübner
Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley:
> On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote:
> > Add the documentation for VOP2 video ports reset clocks.
> > One reset can be set per video port.
> > 
> > Signed-off-by: Detlev Casanova 
> 
> Are these resets valid for all VOPs or just the one on 3588?

Not in that form.
I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2).

So the binding should take into account that rk3568 also has the
SRST_VOP0 ... SRST_VOP2.


Also, I guess we might not want to limit ourself to stuff we use?
I.e. the new VOP-design is one block with multiple video-ports

So for rk3568 I see
#define SRST_A_VOP
#define SRST_H_VOP
#define SRST_VOP0
#define SRST_VOP1
#define SRST_VOP2

similarly rk3588 has

#define SRST_H_VOP
#define SRST_A_VOP
#define SRST_D_VOP0
#define SRST_D_VOP1
#define SRST_D_VOP2
#define SRST_D_VOP3

as generalized reset lines.





> 
> > ---
> >  .../display/rockchip/rockchip-vop2.yaml   | 27 +++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml 
> > b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > index 2531726af306b..941fd059498d4 100644
> > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> > @@ -65,6 +65,22 @@ properties:
> >- const: dclk_vp3
> >- const: pclk_vop
> >  
> > +  resets:
> > +minItems: 3
> > +items:
> > +  - description: Pixel clock reset for video port 0.
> > +  - description: Pixel clock reset for video port 1.
> > +  - description: Pixel clock reset for video port 2.
> > +  - description: Pixel clock reset for video port 3.
> > +
> > +  reset-names:
> > +minItems: 3
> > +items:
> > +  - const: dclk_vp0
> > +  - const: dclk_vp1
> > +  - const: dclk_vp2
> > +  - const: dclk_vp3
> > +
> >rockchip,grf:
> >  $ref: /schemas/types.yaml#/definitions/phandle
> >  description:
> > @@ -128,6 +144,11 @@ allOf:
> >  clock-names:
> >minItems: 7
> >  
> > +resets:
> > +  minItems: 4
> > +reset-names:
> > +  minItems: 4
> > +
> >  ports:
> >required:
> >  - port@0
> > @@ -183,6 +204,12 @@ examples:
> >"dclk_vp0",
> >"dclk_vp1",
> >"dclk_vp2";
> > +resets = <&cru SRST_VOP0>,
> > + <&cru SRST_VOP1>,
> > + <&cru SRST_VOP2>;
> > +reset-names = "dclk_vp0",
> > +  "dclk_vp1",
> > +  "dclk_vp2";
> >  power-domains = <&power RK3568_PD_VO>;
> >  iommus = <&vop_mmu>;
> >  vop_out: ports {
> 






Re: [PATCH v2 0/7] Add DSI support for RK3128

2024-05-09 Thread Heiko Stübner
Hi Alex,

Am Donnerstag, 9. Mai 2024, 14:07:08 CEST schrieb Alex Bee:
> This series aims to add support for the DesignWare MIPI DSI controller and
> the Innoslicon D-PHY found in RK3128 SoCs. The code additions are rather
> tiny: It only need some code in the Rockchip dw-mipi-dsi glue layer for
> this SoC, add support for an additional clock and do some changes in the
> SoC's clock driver. Support for the phy was already added when the
> Innosilicon D-PHY driver was initially submitted. I tested it with a
> 800x1280 DSI panel where all 4 lanes that are supported are used.
> 
> changes in v2:
>   To improve power-efficiency when the DSI controller is not in use, I
>   dropped the patch which made hclk_vio_h2p a critical clock and instead
>   added support for an AHB clock to the DSI controller driver and updated
>   the bindings and the addition to the SoC DT accordingly.

The naming already suggests that hclk_vio_h2p is not a clock-part of
the actual dsi controller, but more an internal thing inside the clock
controller.

At least naming and perceived functionality would suggest a chain of
hclk_vio -> hclk_vio_h2p -> pclk_mipi

In any case, I really don't see hclk_vio_h2p to be in the realm of the
actual DSI controller, but more a part of clock-controller / interconnect.
Similar to the NIU clocks for the interconnect.

rk3588 actually tries to implement this already and while the
gate-link clocks are described as "recent", I think this definitly the same
concept used a most/all older Rockchip SoCs, just nobody cared about that
till now ;-) [0] .

So TL;DR I'd really prefer to not leak CRU-details into the DSI controller.


Heiko

[0] Which reminds me that I should look at Sebastian's make GATE-LINK
actually-work-patch.






Re: [RFT PATCH v2 31/48] drm/panel: xinpeng-xpp055c272: Don't call unprepare+disable at shutdown/remove

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:33:12 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
> 
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
> 
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by Rockchip boards. The Rockchip driver
> appears to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
> 
> [1] https://lore.kernel.org/r/20230901234015.566018-1-diand...@chromium.org
> [2] https://lore.kernel.org/r/20230901234202.566951-1-diand...@chromium.org
> [3] https://lore.kernel.org/r/20230921192749.1542462-1-diand...@chromium.org
> 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 30/48] drm/panel: xinpeng-xpp055c272: Stop tracking prepared

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:33:11 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
> 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 17/48] drm/panel: ltk500hd1829: Don't call unprepare+disable at shutdown/remove

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:58 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
> 
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
> 
> Unfortunately, grepping mainline for this panel's compatible string
> shows no hits, so we can't be 100% sure if the DRM modeset driver used
> with this panel has been fixed. If it is found that the DRM modeset
> driver hasn't been fixed then this patch could be temporarily reverted
> until it is.
> 
> [1] https://lore.kernel.org/r/20230901234015.566018-1-diand...@chromium.org
> [2] https://lore.kernel.org/r/20230901234202.566951-1-diand...@chromium.org
> [3] https://lore.kernel.org/r/20230921192749.1542462-1-diand...@chromium.org
> 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 16/48] drm/panel: ltk500hd1829: Stop tracking prepared

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:57 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
> 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 13/48] drm/panel: kingdisplay-kd097d04: Don't call unprepare+disable at shutdown/remove

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:54 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
> 
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
> 
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by Rockchip boards. The Rockchip driver
> appear to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
> 
> [1] https://lore.kernel.org/r/20230901234015.566018-1-diand...@chromium.org
> [2] https://lore.kernel.org/r/20230901234202.566951-1-diand...@chromium.org
> [3] https://lore.kernel.org/r/20230921192749.1542462-1-diand...@chromium.org
> 
> Cc: Brian Norris 
> Cc: Chris Zhong 
> Cc: Nickey Yang 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 12/48] drm/panel: kingdisplay-kd097d04: Stop tracking prepared/enabled

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:53 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
> 
> Cc: Brian Norris 
> Cc: Chris Zhong 
> Cc: Nickey Yang 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 09/48] drm/panel: innolux-p079zca: Don't call unprepare+disable at shutdown/remove

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:50 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
> 
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
> 
> A grep through mainline for compatible strings used by this driver
> indicates that it is used by Rockchip boards. The Rockchip driver
> appears to be correctly calling drm_atomic_helper_shutdown() so we can
> remove the calls.
> 
> [1] https://lore.kernel.org/r/20230901234015.566018-1-diand...@chromium.org
> [2] https://lore.kernel.org/r/20230901234202.566951-1-diand...@chromium.org
> [3] https://lore.kernel.org/r/20230921192749.1542462-1-diand...@chromium.org
> 
> Cc: Chris Zhong 
> Cc: Lin Huang 
> Cc: Brian Norris 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 08/48] drm/panel: innolux-p079zca: Stop tracking prepared/enabled

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:49 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
> 
> Cc: Chris Zhong 
> Cc: Lin Huang 
> Cc: Brian Norris 
> Cc: "Heiko Stübner" 
> Signed-off-by: Douglas Anderson 

the underlying setup (rockchip-drm with dw-dsi) as well as the
change itself is similar to the ltk050h3146w variant, so I don't
see how this should behave differently ;-)

Reviewed-by: Heiko Stuebner 




Re: [RFT PATCH v2 14/48] drm/panel: ltk050h3146w: Stop tracking prepared

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:55 CEST schrieb Douglas Anderson:
> As talked about in commit d2aacaf07395 ("drm/panel: Check for already
> prepared/enabled in drm_panel"), we want to remove needless code from
> panel drivers that was storing and double-checking the
> prepared/enabled state. Even if someone was relying on the
> double-check before, that double-check is now in the core and not
> needed in individual drivers.
> 
> Cc: "Heiko Stübner" 
> Cc: Quentin Schulz 
> Signed-off-by: Douglas Anderson 

Reviewed-by: Heiko Stuebner 

on a rk3588-tiger with WIP DSI patches
Tested-by: Heiko Stuebner 




Re: [RFT PATCH v2 15/48] drm/panel: ltk050h3146w: Don't call unprepare+disable at shutdown/remove

2024-05-06 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 23:32:56 CEST schrieb Douglas Anderson:
> It's the responsibility of a correctly written DRM modeset driver to
> call drm_atomic_helper_shutdown() at shutdown time and that should be
> disabling / unpreparing the panel if needed. Panel drivers shouldn't
> be calling these functions themselves.
> 
> A recent effort was made to fix as many DRM modeset drivers as
> possible [1] [2] [3] and most drivers are fixed now.
> 
> Unfortunately, grepping mainline for this panel's compatible string
> shows no hits, so we can't be 100% sure if the DRM modeset driver used
> with this panel has been fixed. If it is found that the DRM modeset
> driver hasn't been fixed then this patch could be temporarily reverted
> until it is.
> 
> [1] https://lore.kernel.org/r/20230901234015.566018-1-diand...@chromium.org
> [2] https://lore.kernel.org/r/20230901234202.566951-1-diand...@chromium.org
> [3] https://lore.kernel.org/r/20230921192749.1542462-1-diand...@chromium.org
> 
> Cc: "Heiko Stübner" 
> Cc: Quentin Schulz 
> Signed-off-by: Douglas Anderson 

On a rk3588-tiger with WIP DSI patches and this display

Tested-by: Heiko Stuebner 

Before this patch on reboot the system emits
[  181.464230] panel-leadtek-ltk050h3146w fde2.dsi.0: Skipping disable of 
already disabled panel
[  181.465056] panel-leadtek-ltk050h3146w fde2.dsi.0: Skipping unprepare of 
already unprepared panel

after applying this patch, those lines are gone.

Also the patch itself looks good
Reviewed-by: Heiko Stuebner 




Re: [PATCH 2/2] drm/rockchip: vop2: configure layers for vp3 on rk3588

2024-05-03 Thread Heiko Stübner
Am Freitag, 3. Mai 2024, 14:57:03 CEST schrieb Quentin Schulz:
> Hi Heiko,
> 
> On 4/25/24 9:55 PM, Heiko Stuebner wrote:
> > From: Heiko Stuebner 
> > 
> > The rk3588 VOP2 has 4 video-ports, yet the driver currently only
> > configures the first 3, as used on the rk3568.
> > 
> 
> I'm wondering whether we should update the drawing at the top of the 
> driver then?
> 
> > Add another block to configure the vp3 as well, if applicable.
> > 
> > Fixes: 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588")
> > Signed-off-by: Heiko Stuebner 
> > ---
> >   drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 12 
> >   drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  1 +
> >   2 files changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c 
> > b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > index 523880a4e8e74..1a327a9ed7ee4 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > @@ -2303,6 +2303,7 @@ static void vop2_setup_alpha(struct vop2_video_port 
> > *vp)
> >   static void vop2_setup_layer_mixer(struct vop2_video_port *vp)
> >   {
> > struct vop2 *vop2 = vp->vop2;
> > +   const struct vop2_data *vop2_data = vop2->data;
> > struct drm_plane *plane;
> > u32 layer_sel = 0;
> > u32 port_sel;
> > @@ -2344,6 +2345,17 @@ static void vop2_setup_layer_mixer(struct 
> > vop2_video_port *vp)
> > else
> > port_sel |= FIELD_PREP(RK3568_OVL_PORT_SET__PORT2_MUX, 8);
> >   
> > +   /* configure vp3 */
> > +   if (vop2_data->soc_id == 3588) {
> 
> I think it'd be smarter to check against vop2->data->nr_vps >= 4; so 
> that we don't need to maintain a list of SoCs that support a specific 
> number of video ports.

probably ;-)

> 
> > +   struct vop2_video_port *vp3 = &vop2->vps[3];
> 
> This is always possible because vps is statically allocated for 4 items, 
> c.f. struct vop2_video_port vps[ROCKCHIP_MAX_CRTC]; so we don't 
> necessarily need it in this specific location and can group it with the 
> others. Cosmetic suggestion though.
> 
> Otherwise, the change itself makes sense to me, so:
> 
> Reviewed-by: Quentin Schulz 

though comments from Andy from Rockchip in another thread suggest that
this is not necessary at all, as the "last" vp somehow has a hardware lock
to take the remaining layers or so.

And while tracking down dsi issues I had a "binary" state of
"gray display" without this patch and working DSI with it, in the last days
I haven't been able to reproduce this anymore.

So I guess I'll fix up the first patch according to your comment and keep
this change here for later.


Heiko




Re: [PATCH 1/1] drm/rockchip: vop2: Fix the port mux of VP2

2024-04-25 Thread Heiko Stübner
Am Montag, 22. April 2024, 12:19:05 CEST schrieb Andy Yan:
> From: Andy Yan 
> 
> The port mux of VP2 should be RK3568_OVL_PORT_SET__PORT2_MUX.
> 
> Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> Signed-off-by: Andy Yan 

on a rk3588 with VP3 connected to a DSI display
Tested-by: Heiko Stuebner 





Re: [PATCH] dt-bindings: display: rockchip: add missing #sound-dai-cells to dw-hdmi

2024-03-26 Thread Heiko Stübner
Am Dienstag, 26. März 2024, 18:50:37 CET schrieb Krzysztof Kozlowski:
> On 26/03/2024 18:50, Krzysztof Kozlowski wrote:
> > On 26/03/2024 18:28, Heiko Stuebner wrote:
> >> The #sound-dai-cells DT property is required to describe link between
> >> the HDMI IP block and the SoC's audio subsystem.
> >>
> >> Signed-off-by: Heiko Stuebner 
> >> ---
> >>  .../devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git 
> >> a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml 
> >> b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> >> index af638b6c0d21..3768df80ca7a 100644
> >> --- 
> >> a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> >> +++ 
> >> b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
> >> @@ -124,6 +124,9 @@ properties:
> >>  description:
> >>phandle to the GRF to mux vopl/vopb.
> >>  
> >> +  "#sound-dai-cells":
> >> +const: 0
> >> +
> > 
> > Then you miss $ref in allOf to /schemas/sound/dai-common.yaml
> 
> I meant, except your change you should add also above $ref.

sorry about that, will fix that.

Thanks for the pointer
Heiko





Re: [PATCH v5 00/14] drm: Add a driver for CSF-based Mali GPUs

2024-02-28 Thread Heiko Stübner
Am Sonntag, 18. Februar 2024, 22:41:14 CET schrieb Boris Brezillon:
> Hello,
> 
> This is the 5th version of the kernel driver for Mali CSF-based GPUs,
> and, unless someone has good reasons to block the merging of this
> driver, I expect it to be the last one (the gallium driver is now
> in a decent state, and is mostly waiting for the kernel driver to
> be accepted).
> 
> A branch based on drm-misc-next is available here[1], and here is
> another one [2] containing extra patches to have things working on
> rk3588. The CSF firmware binary is now merged in linux-firmware [3].
> 
> The mesa MR adding v10 support on top of panthor is available here [4].
> 
> Here is a non-exhaustive changelog, check each commit for a detailed
> changelog.
> 
> v5:
> - No fundamental changes in this v5
> - Various bug fixes (see the per-commit changelogs)
> - Various docs/typos fixes
> 
> v4:
> - Fix various bugs in the VM logic
> - Address comments from Steven, Liviu, Ketil and Chris
> - Move tiler OOM handling out of the scheduler interrupt handling path
>   so we can properly recover when the system runs out of memory, and
>   panthor is blocked trying to allocate heap chunks
> - Rework the heap locking to support concurrent chunk allocation. Not
>   sure if this is supposed to happen, but we need to be robust against
>   userspace passing the same heap context to two scheduling groups.
>   Wasn't needed before the tiler_oom rework, because heap allocation
>   base serialized by the scheduler lock.
> - Make kernel BO destruction robust to NULL/ERR pointers
> 
> v3;
> - Quite a few changes at the MMU/sched level to make the fix some
>   race conditions and deadlocks
> - Addition of the a sync-only VM_BIND operation (to support
>   vkQueueSparseBind with zero commands).
> - Addition of a VM_GET_STATE ioctl
> 
> [1]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v5
> [2]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v5+rk3588
> [3]https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/arm/mali/arch10.8
> [4]https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26358
> 
> Boris Brezillon (13):
>   drm/panthor: Add uAPI
>   drm/panthor: Add GPU register definitions
>   drm/panthor: Add the device logical block
>   drm/panthor: Add the GPU logical block
>   drm/panthor: Add GEM logical block
>   drm/panthor: Add the devfreq logical block
>   drm/panthor: Add the MMU/VM logical block
>   drm/panthor: Add the FW logical block
>   drm/panthor: Add the heap logical block
>   drm/panthor: Add the scheduler logical block
>   drm/panthor: Add the driver frontend block
>   drm/panthor: Allow driver compilation
>   drm/panthor: Add an entry to MAINTAINERS

on a rk3588-jaguar with pending hdmi patches
Tested-by: Heiko Stuebner 

Also the series looks nice to my cursory glance, so
Acked-by: Heiko Stuebner 




Re: [PATCH v7 31/36] drm/rockchip: inno_hdmi: Switch to HDMI connector

2024-02-23 Thread Heiko Stübner
Am Donnerstag, 22. Februar 2024, 19:14:17 CET schrieb Maxime Ripard:
> The new HDMI connector infrastructure allows to remove some boilerplate,
> especially to generate infoframes. Let's switch to it.
> 
> Signed-off-by: Maxime Ripard 

Reviewed-by: Heiko Stuebner 

> ---
>  drivers/gpu/drm/rockchip/inno_hdmi.c | 123 
> ---
>  1 file changed, 42 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
> b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 1d2261643743..d59947679042 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -67,9 +67,7 @@ struct inno_hdmi {
>  
>  struct inno_hdmi_connector_state {
>   struct drm_connector_state  base;
> - unsigned intenc_out_format;
>   unsigned intcolorimetry;
> - boolrgb_limited_range;
>  };
>  
>  static struct inno_hdmi *encoder_to_inno_hdmi(struct drm_encoder *encoder)
> @@ -257,26 +255,29 @@ static void inno_hdmi_reset(struct inno_hdmi *hdmi)
>   inno_hdmi_standby(hdmi);
>  }
>  
> -static void inno_hdmi_disable_frame(struct inno_hdmi *hdmi,
> - enum hdmi_infoframe_type type)
> +static int inno_hdmi_disable_frame(struct drm_connector *connector,
> +enum hdmi_infoframe_type type)
>  {
> - struct drm_connector *connector = &hdmi->connector;
> + struct inno_hdmi *hdmi = connector_to_inno_hdmi(connector);
>  
>   if (type != HDMI_INFOFRAME_TYPE_AVI) {
>   drm_err(connector->dev,
>   "Unsupported infoframe type: %u\n", type);
> - return;
> + return 0;
>   }
>  
>   hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_BUF_INDEX, INFOFRAME_AVI);
> +
> + return 0;
>  }
>  
> -static int inno_hdmi_upload_frame(struct inno_hdmi *hdmi,
> -   union hdmi_infoframe *frame, enum 
> hdmi_infoframe_type type)
> +static int inno_hdmi_upload_frame(struct drm_connector *connector,
> +   enum hdmi_infoframe_type type,
> +   const u8 *buffer, size_t len)
>  {
> - struct drm_connector *connector = &hdmi->connector;
> + struct inno_hdmi *hdmi = connector_to_inno_hdmi(connector);
>   u8 packed_frame[HDMI_MAXIMUM_INFO_FRAME_SIZE];
> - ssize_t rc, i;
> + ssize_t i;
>  
>   if (type != HDMI_INFOFRAME_TYPE_AVI) {
>   drm_err(connector->dev,
> @@ -284,59 +285,19 @@ static int inno_hdmi_upload_frame(struct inno_hdmi 
> *hdmi,
>   return 0;
>   }
>  
> - inno_hdmi_disable_frame(hdmi, type);
> + inno_hdmi_disable_frame(connector, type);
>  
> - rc = hdmi_infoframe_pack(frame, packed_frame,
> -  sizeof(packed_frame));
> - if (rc < 0)
> - return rc;
> -
> - for (i = 0; i < rc; i++)
> + for (i = 0; i < len; i++)
>   hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_ADDR + i,
>   packed_frame[i]);
>  
>   return 0;
>  }
>  
> -static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
> -   struct drm_display_mode *mode)
> -{
> - struct drm_connector *connector = &hdmi->connector;
> - struct drm_connector_state *conn_state = connector->state;
> - struct inno_hdmi_connector_state *inno_conn_state =
> - to_inno_hdmi_conn_state(conn_state);
> - union hdmi_infoframe frame;
> - int rc;
> -
> - rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> -   &hdmi->connector,
> -   mode);
> - if (rc) {
> - inno_hdmi_disable_frame(hdmi, HDMI_INFOFRAME_TYPE_AVI);
> - return rc;
> - }
> -
> - if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV444)
> - frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> - else if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_YUV422)
> - frame.avi.colorspace = HDMI_COLORSPACE_YUV422;
> - else
> - frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> -
> - if (inno_conn_state->enc_out_format == HDMI_COLORSPACE_RGB) {
> - drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> -connector, mode,
> -
> inno_conn_state->rgb_limited_range ?
> -
> HDMI_QUANTIZATION_RANGE_LIMITED :
> -
> HDMI_QUANTIZATION_RANGE_FULL);
> - } else {
> - frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> - frame.avi.ycc_quantization_range =
> - HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
> - }
> -
> - return inno_hdmi_upload_fr

Re: [PATCH 2/3] dt-bindings: display: ltk500hd1829: add variant compatible for ltk101b4029w

2024-02-15 Thread Heiko Stübner
Am Donnerstag, 15. Februar 2024, 18:06:06 CET schrieb Conor Dooley:
> On Thu, Feb 15, 2024 at 10:05:14AM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner 
> > 
> > Add the compatible for the ltk101b4029w panel, that is really similar
> > to the ltk500hd1829.
> 
> Please mention what makes the devices incompatible. "really similar" is
> vague and could be used for a device that was only cosmetically
> different.

ok, I'll modify the paragraph to:

===
Add the compatible for the ltk101b4029w panel, that has the same
manufacturer, general bringup and supplies but a different dsi-init-
sequence as the ltk500hd1829 .
===

> With that,
> Acked-by: Conor Dooley 
> 
> Cheers,
> Conor.
> 
> > 
> > Signed-off-by: Heiko Stuebner 
> > ---
> >  .../bindings/display/panel/leadtek,ltk500hd1829.yaml  | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/display/panel/leadtek,ltk500hd1829.yaml 
> > b/Documentation/devicetree/bindings/display/panel/leadtek,ltk500hd1829.yaml
> > index c5944b4d636c5..d589f16772145 100644
> > --- 
> > a/Documentation/devicetree/bindings/display/panel/leadtek,ltk500hd1829.yaml
> > +++ 
> > b/Documentation/devicetree/bindings/display/panel/leadtek,ltk500hd1829.yaml
> > @@ -14,7 +14,9 @@ allOf:
> >  
> >  properties:
> >compatible:
> > -const: leadtek,ltk500hd1829
> > +enum:
> > +  - leadtek,ltk101b4029w
> > +  - leadtek,ltk500hd1829
> >reg: true
> >backlight: true
> >reset-gpios: true
> 






Re: [PATCH v4 0/4] arm64: rockchip: Pine64 PineTab2 support

2024-01-31 Thread Heiko Stübner
Am Dienstag, 30. Januar 2024, 20:36:22 CET schrieb Manuel Traut:
> Hi Dang,
> 
> On Sat, Jan 27, 2024 at 06:35:50PM +0700, Dang Huynh wrote:
> > Hi Manuel,
> > 
> > Since the BOE patches have been accepted to next, you do not need to 
> > include 
> > it in this patch series.
> 
> sorry, I thought patches to LKML shall be against Linux master since the
> patches are still only in drm-next I considered to keep them in the queue.

normally if parts of the v(x-1) version of your series are already
applied somewhere you just mention that fact in the cover-letter
in the changelog, like "dropped display patches already applied to drm-misc"

Just to make sure, there is no need for a resend, I'll look at and pick
just the remaining patches :-)


Heiko




Re: [PATCH v4 11/14] drm/panthor: Add the driver frontend block

2024-01-23 Thread Heiko Stübner
Am Montag, 22. Januar 2024, 17:30:42 CET schrieb Boris Brezillon:
> This is the last piece missing to expose the driver to the outside
> world.
> 
> This is basically a wrapper between the ioctls and the other logical
> blocks.
> 
> v4:
> - Add an ioctl to let the UMD query the VM state
> - Fix kernel doc
> - Let panthor_device_init() call panthor_device_init()
> - Fix cleanup ordering in the panthor_init() error path
> - Add Steve's and Liviu's R-b
> 
> v3:
> - Add acks for the MIT/GPL2 relicensing
> - Fix 32-bit support
> - Account for panthor_vm and panthor_sched changes
> - Simplify the resv preparation/update logic
> - Use a linked list rather than xarray for list of signals.
> - Simplify panthor_get_uobj_array by returning the newly allocated
>   array.
> - Drop the "DOC" for job submission helpers and move the relevant
>   comments to panthor_ioctl_group_submit().
> - Add helpers sync_op_is_signal()/sync_op_is_wait().
> - Simplify return type of panthor_submit_ctx_add_sync_signal() and
>   panthor_submit_ctx_get_sync_signal().
> - Drop WARN_ON from panthor_submit_ctx_add_job().
> - Fix typos in comments.
> 
> Co-developed-by: Steven Price 
> Signed-off-by: Steven Price 
> Signed-off-by: Boris Brezillon 
> Acked-by: Steven Price  # MIT+GPL2 relicensing,Arm
> Acked-by: Grant Likely  # MIT+GPL2 relicensing,Linaro
> Acked-by: Boris Brezillon  # MIT+GPL2 
> relicensing,Collabora
> Reviewed-by: Steven Price 
> Reviewed-by: Liviu Dudau 
> ---
>  drivers/gpu/drm/panthor/panthor_drv.c | 1470 +
>  1 file changed, 1470 insertions(+)
>  create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c 
> b/drivers/gpu/drm/panthor/panthor_drv.c
> new file mode 100644
> index ..207aacaccd39
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
> @@ -0,0 +1,1470 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +/* Copyright 2018 Marty E. Plummer  */
> +/* Copyright 2019 Linaro, Ltd., Rob Herring  */
> +/* Copyright 2019 Collabora ltd. */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 

with v6.8-rc1 this needs a linux/platform_device.h include to keep
finding struct platform_device and friends

[...]


> +static int panthor_submit_ctx_init(struct panthor_submit_ctx *ctx,
> +struct drm_file *file, u32 job_count)
> +{
> + ctx->jobs = kvmalloc_array(job_count, sizeof(*ctx->jobs),
> +GFP_KERNEL | __GFP_ZERO);
> + if (!ctx->jobs)
> + return -ENOMEM;
> +
> + ctx->file = file;
> + ctx->job_count = job_count;
> + INIT_LIST_HEAD(&ctx->signals);
> + drm_exec_init(&ctx->exec, DRM_EXEC_INTERRUPTIBLE_WAIT | 
> DRM_EXEC_IGNORE_DUPLICATES);

../drivers/gpu/drm/panthor/panthor_drv.c: In function ‘panthor_submit_ctx_init’:
../drivers/gpu/drm/panthor/panthor_drv.c:722:9: error: too few arguments to 
function ‘drm_exec_init’
  722 | drm_exec_init(&ctx->exec, DRM_EXEC_INTERRUPTIBLE_WAIT | 
DRM_EXEC_IGNORE_DUPLICATES);
  | ^

In v6.8-rc1 (or drm-misc-next I guess) the calling convention of
drm_exec_init changed to include a number of initial objects, see
commit 05d249352f1a ("drm/exec: Pass in initial # of objects")


Heiko




Re: [PATCH v4 00/14] drm: Add a driver for CSF-based Mali GPUs

2024-01-23 Thread Heiko Stübner
Am Montag, 22. Januar 2024, 17:30:31 CET schrieb Boris Brezillon:
> Hello,
> 
> This is the 4th version of the kernel driver for Mali CSF-based GPUs.
> 
> A branch based on drm-misc-next and containing all the dependencies
> that are not yet available in drm-misc-next here[1], and another [2]
> containing extra patches to have things working on rk3588. The CSF
> firmware binary can be found here[3], and should be placed under
> /lib/firmware/arm/mali/arch10.8/mali_csffw.bin.
> 
> The mesa MR adding v10 support on top of panthor is available here [4].
> 
> Steve, I intentionally dropped your R-b on "drm/panthor: Add the heap
> logical block" and "drm/panthor: Add the scheduler logical block"
> because the tiler-OOM handling changed enough to require a new review
> IMHO.
> 
> Regarding the GPL2+MIT relicensing, I collected Clément's R-b for the
> devfreq code, but am still lacking Alexey Sheplyakov for some bits in
> panthor_gpu.c. The rest of the code is either new, or covered by the
> Linaro, Arm and Collabora acks.

On a rk3588-jaguar with a corresponding mesa build, series:
Tested-by: Heiko Stuebner 


Thanks for all the work on this
Heiko




Re: (subset) [PATCH] drm/rockchip: inno_hdmi: Explicitly include drm_atomic.h

2024-01-17 Thread Heiko Stübner
Am Mittwoch, 17. Januar 2024, 14:47:48 CET schrieb Maxime Ripard:
> On Wed, Jan 17, 2024 at 10:52:04AM +0100, Heiko Stübner wrote:
> > Hi Maxime,
> > 
> > Am Mittwoch, 17. Januar 2024, 10:46:57 CET schrieb Maxime Ripard:
> > > On Mon, 15 Jan 2024 10:24:35 +0100, Alex Bee wrote:
> > > > Commit d3e040f450ec ("drm/rockchip: inno_hdmi: Get rid of mode_set")
> > > > started using drm_atomic_get_new_connector_state and
> > > > drm_atomic_get_new_crtc_state which are defined in drm_atomic.h
> > > > Building does currently only work if CONFIG_OF and 
> > > > CONFIG_DRM_PANEL_BRIDGE
> > > > are enabled since this will include drm_atomic.h via drm_bridge.h (see
> > > > drm_of.h).
> > > > 
> > > > [...]
> > > 
> > > Applied to drm/drm-misc (drm-misc-next).
> > 
> > wouldn't have drm-misc-next-fixes been more appropriate?
> > Because I _think_ the change causing the issue made it in during the
> > current merge-window?
> 
> AFAIK, the offending commit is in drm-misc-next only

ah, you're of course right. Mistook that for the one in the rk3066_hdmi
but that was fixed back in november already.

So all is well.

Heiko




Re: (subset) [PATCH] drm/rockchip: inno_hdmi: Explicitly include drm_atomic.h

2024-01-17 Thread Heiko Stübner
Hi Maxime,

Am Mittwoch, 17. Januar 2024, 10:46:57 CET schrieb Maxime Ripard:
> On Mon, 15 Jan 2024 10:24:35 +0100, Alex Bee wrote:
> > Commit d3e040f450ec ("drm/rockchip: inno_hdmi: Get rid of mode_set")
> > started using drm_atomic_get_new_connector_state and
> > drm_atomic_get_new_crtc_state which are defined in drm_atomic.h
> > Building does currently only work if CONFIG_OF and CONFIG_DRM_PANEL_BRIDGE
> > are enabled since this will include drm_atomic.h via drm_bridge.h (see
> > drm_of.h).
> > 
> > [...]
> 
> Applied to drm/drm-misc (drm-misc-next).

wouldn't have drm-misc-next-fixes been more appropriate?
Because I _think_ the change causing the issue made it in during the
current merge-window?

Heiko




Re: [PATCH] drm/bridge: synopsys: dw-mipi-dsi: fix deferred dsi host probe breaks dsi device probe

2024-01-15 Thread Heiko Stübner
Am Montag, 15. Januar 2024, 09:45:10 CET schrieb neil.armstr...@linaro.org:
> Hi,
> 
> On 12/01/2024 19:07, Farouk Bouabid wrote:
> > dw-mipi-dsi based drivers such as dw-mipi-dsi-rockchip or dw_mipi_dsi-stm
> > depend on dw_mipi_dsi_probe() to initialize the dw_mipi_dsi driver
> > structure (dmd pointer). This structure is only initialized once
> > dw_mipi_dsi_probe() returns, creating the link between the locally created
> > structure and the actual dmd pointer.
> > 
> > Probing the dsi host can be deferred in case of dependency to a dsi
> > phy-supply (eg. "rockchip,px30-dsi-dphy"). Meanwhile dsi-device drivers
> > like panels (eg. "ltk050h3146w") can already be registered on the bus.
> > In that case, when attempting, to register the dsi host from
> > dw_mipi_dsi_probe() using mipi_dsi_host_register(), the panel probe is
> > called with a dsi-host pointer that is still locally allocated in
> > dw_mipi_dsi_probe().
> > 
> > While probing, the panel driver tries to attach to a dsi host
> > (mipi_dsi_attach()) which calls in return for the specific dsi host
> > attach hook. (e.g. dw_mipi_dsi_rockchip_host_attach()).
> > dw_mipi_dsi_rockchip uses the component framework.
> > In the attach hook, the host component is registered which calls in return
> > for drm_bridge_attach() while trying to bind the component
> > (dw_mipi_dsi_bind())
> 
> In meson_dw_mipi_dsi I simply fixed this by getting rid of components...

If I remember correctly, the component element is there on Rockchip to
facilitate running dual-dsi displays (1 panel driven by 2 dsi controllers),
because it allows to wait for both controllers to have probed individually
before trying to drive the display.

Not sure if there is a better way to do that now.


Heiko


> > drm_bridge_attach() requires a valid drm bridge parameter. However, the
> > drm bridge (&dmd->bridge) that will be passed, is not yet initialized since
> > the dw_mipi_dsi_probe() has not yet returned. This call will fail with a
> > fatal error (invalid bridge) causing the panel to not be probed again.
> > 
> > To simplify the issue: drm_bridge_attach() depends on the result pointer
> > of dw_mipi_dsi_probe().
> > While, if the dsi probe is deferred, drm_bridge_attach() is called before
> > dw_mipi_dsi_probe() returns.
> > 
> > drm_bridge_attach+0x14/0x1ac
> > dw_mipi_dsi_bind+0x24/0x30
> > dw_mipi_dsi_rockchip_bind+0x258/0x378
> > component_bind_all+0x118/0x248
> > rockchip_drm_bind+0xb0/0x1f8
> > try_to_bring_up_aggregate_device+0x168/0x1d4
> > __component_add+0xa4/0x170
> > component_add+0x14/0x20
> > dw_mipi_dsi_rockchip_host_attach+0x54/0x144
> > dw_mipi_dsi_host_attach+0x9c/0xcc
> > mipi_dsi_attach+0x28/0x3c
> > ltk050h3146w_probe+0x10c/0x1a4
> > mipi_dsi_drv_probe+0x20/0x2c
> > really_probe+0x148/0x2ac
> > __driver_probe_device+0x78/0x12c
> > driver_probe_device+0xdc/0x160
> > __device_attach_driver+0xb8/0x134
> > bus_for_each_drv+0x80/0xdc
> > __device_attach+0xa8/0x1b0
> > device_initial_probe+0x14/0x20
> > bus_probe_device+0xa8/0xac
> > device_add+0x5cc/0x778
> > mipi_dsi_device_register_full+0xd8/0x198
> > mipi_dsi_host_register+0x98/0x18c
> > __dw_mipi_dsi_probe+0x290/0x35c
> > dw_mipi_dsi_probe+0x10/0x6c
> > dw_mipi_dsi_rockchip_probe+0x208/0x3e4
> > platform_probe+0x68/0xdc
> > really_probe+0x148/0x2ac
> > __driver_probe_device+0x78/0x12c
> > driver_probe_device+0xdc/0x160
> > __device_attach_driver+0xb8/0x134
> > bus_for_each_drv+0x80/0xdc
> > __device_attach+0xa8/0x1b0
> > device_initial_probe+0x14/0x20
> > bus_probe_device+0xa8/0xac
> > deferred_probe_work_func+0x88/0xc0
> > process_one_work+0x138/0x260
> > worker_thread+0x32c/0x438
> > kthread+0x118/0x11c
> > ret_from_fork+0x10/0x20
> > ---[ end trace  ]---
> > 
> > Fix this by initializing directly the dmd pointer in dw_mipi_dsi_probe(),
> > which requires also initializting the dmd->bridge attributes that are
> > required in drm_bridge_attach() before calling mipi_dsi_host_register().
> > 
> > Signed-off-by: Farouk Bouabid 
> > ---
> >   drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c   |  4 +-
> >   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 42 ++-
> >   drivers/gpu/drm/meson/meson_dw_mipi_dsi.c |  8 ++--
> >   .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   |  5 +--
> >   drivers/gpu/drm/stm/dw_mipi_dsi-stm.c |  5 +--
> >   include/drm/bridge/dw_mipi_dsi.h  |  5 ++-
> >   6 files changed, 35 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c 
> > b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > index 3ff30ce80c5b..469976ad3b19 100644
> > --- a/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > +++ b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> > @@ -881,8 +881,8 @@ static int imx93_dsi_probe(struct platform_device *pdev)
> > dsi->pdata.priv_data = dsi;
> > platform_set_drvdata(pdev, dsi);
> >   
> > -   dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
> > -   if (IS_ERR(dsi->dmd))
> > +   ret = dw_mipi_d

Re: (subset) [PATCH v4 00/29] Add HDMI support for RK3128

2024-01-05 Thread Heiko Stübner
Am Freitag, 5. Januar 2024, 18:33:34 CET schrieb Alex Bee:
> 
> Am 05.01.24 um 18:02 schrieb Heiko Stübner:
> > Am Freitag, 5. Januar 2024, 17:47:21 CET schrieb Alex Bee:
> >> Hi Heiko,
> >>
> >>
> >> Am 04.01.24 um 09:14 schrieb Heiko Stuebner:
> >>> On Fri, 22 Dec 2023 18:41:51 +0100, Alex Bee wrote:
> >>>> This is version 4 of my series that aims to add support for the display
> >>>> controller (VOP) and the HDMI controller block of RK3128 (which is very
> >>>> similar to the one found in RK3036). The original intention of this 
> >>>> series
> >>>> was to add support for this slightly different integration but is by now,
> >>>> driven by maintainer's feedback, exploded to be a rework of inno-hdmi
> >>>> driver in large parts. It is, however, a change for the better.
> >>>>
> >>>> [...]
> >>> Applied, thanks!
> >>>
> >>> [23/29] drm/rockchip: inno_hdmi: Add variant support
> >>>   commit: 5f2e93e6719701a91307090f8f7696fd6b3bffdf
> >>> [24/29] drm/rockchip: inno_hdmi: Add RK3128 support
> >>>   commit: aa54f334c291effe321aa4b9ac0e67a895fd7b58
> >>> [25/29] drm/rockchip: inno_hdmi: Add basic mode validation
> >>>   commit: 701029621d4141d0c9f8b81a88a37b95ec84ce65
> >>> [26/29] drm/rockchip: inno_hdmi: Drop custom fill_modes hook
> >>>   commit: 50a3c772bd927dd409c484832ddd9f6bf00b7389
> >>>
> >>>
> >>> For reference, Rob has applied the rk3128 compatible in
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/commit/?id=21960bda59852ca961fcd27fba9f92750caccd06
> >> thanks for keeping track on this.
> >>
> >> Is there any reason the DT paches aren't merged yet? From what I can see
> >> they should be fine to be merged in your v6.8-armsoc/dts32 branch which is
> >> 6.7-rc1 based. There was only a txt-binding at this point and it's very
> >> likely that both the rockchip,inno-hdmi.yaml-conversion and the rk3128
> >> additon will both land in 6.8 (they are both in linux-next). Linus' 6.8
> >> merge-window will open earliest next week.
> > Exactly ... and the arm subarchitectures (Rockchip, etc) feed into the
> > more generic soc-tree[0]  and from there in a set of pull requests.
> >
> > Normally everything needs to go to the soc tree before -rc7 .
> > With the whole xmas stuff, I sent some stragglers in a second pull
> > request on monday, but that was already before Rob applied the
> > binding on tuesday.
> >
> > So 6.8 devicetree stuff is essentially done and the dts patches
> > from this series will go in to 6.9 .
> >
> >
> > Hope that explains things a bit :-)
> I assumed (for some reason) that sub-architecture maintainers are allowed
> to send PRs to the respective upper tree until the merge window opens and
> "all the rest" is done within this  ~2 weeks.
> Thanks for explaining.

No worries :-) .

The general rule of thumb is that everything should be done and ready
before the merge-window opens. Linus often writes very positively about
people sending him pull-requests even before the merge window opens ;-)
[meaning their tree is settled early and all test-robots have run]

And there are different rules in every tree.

For the soc tree the general rule of thumb of =< -rc7 - earlier with larger
changesets. On the other side drm-misc stays open all the time, but makes
a cut at -rc6. So everything targetted at v6.8 needs to be in before
v6.7-rc6.


Heiko




Re: (subset) [PATCH v4 00/29] Add HDMI support for RK3128

2024-01-05 Thread Heiko Stübner
Am Freitag, 5. Januar 2024, 17:47:21 CET schrieb Alex Bee:
> Hi Heiko,
> 
> 
> Am 04.01.24 um 09:14 schrieb Heiko Stuebner:
> > On Fri, 22 Dec 2023 18:41:51 +0100, Alex Bee wrote:
> >> This is version 4 of my series that aims to add support for the display
> >> controller (VOP) and the HDMI controller block of RK3128 (which is very
> >> similar to the one found in RK3036). The original intention of this series
> >> was to add support for this slightly different integration but is by now,
> >> driven by maintainer's feedback, exploded to be a rework of inno-hdmi
> >> driver in large parts. It is, however, a change for the better.
> >>
> >> [...]
> > Applied, thanks!
> >
> > [23/29] drm/rockchip: inno_hdmi: Add variant support
> >  commit: 5f2e93e6719701a91307090f8f7696fd6b3bffdf
> > [24/29] drm/rockchip: inno_hdmi: Add RK3128 support
> >  commit: aa54f334c291effe321aa4b9ac0e67a895fd7b58
> > [25/29] drm/rockchip: inno_hdmi: Add basic mode validation
> >  commit: 701029621d4141d0c9f8b81a88a37b95ec84ce65
> > [26/29] drm/rockchip: inno_hdmi: Drop custom fill_modes hook
> >  commit: 50a3c772bd927dd409c484832ddd9f6bf00b7389
> >
> >
> > For reference, Rob has applied the rk3128 compatible in
> > https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/commit/?id=21960bda59852ca961fcd27fba9f92750caccd06
> thanks for keeping track on this.
> 
> Is there any reason the DT paches aren't merged yet? From what I can see
> they should be fine to be merged in your v6.8-armsoc/dts32 branch which is
> 6.7-rc1 based. There was only a txt-binding at this point and it's very
> likely that both the rockchip,inno-hdmi.yaml-conversion and the rk3128
> additon will both land in 6.8 (they are both in linux-next). Linus' 6.8
> merge-window will open earliest next week.

Exactly ... and the arm subarchitectures (Rockchip, etc) feed into the
more generic soc-tree[0]  and from there in a set of pull requests.

Normally everything needs to go to the soc tree before -rc7 .
With the whole xmas stuff, I sent some stragglers in a second pull
request on monday, but that was already before Rob applied the
binding on tuesday.

So 6.8 devicetree stuff is essentially done and the dts patches
from this series will go in to 6.9 .


Hope that explains things a bit :-)
Heiko

[0] https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/

> I'm really not pressuring here and I'm fine if they land in 6.9 - it's just
> for my understanding for further submissions.
> 
> Alex
> 
> >
> > Best regards,
> 






Re: [PATCH] drm/rockchip: vop2: Drop unused if_dclk_rate variable

2024-01-05 Thread Heiko Stübner
Hi,

Am Freitag, 5. Januar 2024, 10:04:55 CET schrieb Andy Yan:
> On 1/4/24 23:58, Heiko Stübner wrote:
> > Am Donnerstag, 4. Januar 2024, 15:39:50 CET schrieb Cristian Ciocaltea:
> >> Commit 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588")
> >> introduced a variable which ended up being unused.  Remove it.
> >>
> >> rockchip_drm_vop2.c:1688:23: warning: variable ‘if_dclk_rate’ set but not 
> >> used [-Wunused-but-set-variable]
> >>
> >> Signed-off-by: Cristian Ciocaltea 
> > 
> > in general, please don't send non-series patches as replies to other 
> > patches.
> > It confuses tooling like b4 way too often, as this patch is not designated
> > as a 2/2 (similar to the first one not being 1/2).
> > 
> >> ---
> >>   drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2 --
> >>   1 file changed, 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c 
> >> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> index 44508c2dd614..923985d4161b 100644
> >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> @@ -1685,7 +1685,6 @@ static unsigned long rk3588_calc_cru_cfg(struct 
> >> vop2_video_port *vp, int id,
> >>unsigned long dclk_core_rate = v_pixclk >> 2;
> >>unsigned long dclk_rate = v_pixclk;
> >>unsigned long dclk_out_rate;
> >> -  unsigned long if_dclk_rate;
> >>unsigned long if_pixclk_rate;
> >>int K = 1;
> >>   
> >> @@ -1700,7 +1699,6 @@ static unsigned long rk3588_calc_cru_cfg(struct 
> >> vop2_video_port *vp, int id,
> >>}
> >>   
> >>if_pixclk_rate = (dclk_core_rate << 1) / K;
> >> -  if_dclk_rate = dclk_core_rate / K;
> >>/*
> >> * *if_pixclk_div = dclk_rate / if_pixclk_rate;
> >> * *if_dclk_div = dclk_rate / if_dclk_rate;
> >> */
> > *if_pixclk_div = 2;
> > *if_dclk_div = 4;
> > 
> > with the code continuing with those static constants but the comment
> > showing a forumula, I do hope Andy can provide a bit of insight into
> > what is happening here.
> > 
> > I.e. I'd really like to understand if that really is just a remnant or
> > something different is needed.
> 
> This is not a remnant, in my V1, I calculate all the div by formula, but 
> Sascha prefer
> more for a constants value[0], so I keep this formula as comments to indicate 
> how these value come from.
> 
> [0]https://patchwork.kernel.org/project/linux-rockchip/patch/20231114112855.1771372-1-andys...@163.com/

thanks for referencing the source of the change.
Leaving the formula in there was the right choice I think

That still leaves the issue with the "unused" warning.

@Christan: in the hdmi block itself can you move the 
if_dclk_rate = dclk_core_rate / K;
to the comment block please? And possibly reference the use
of the static values in the comment message.

The if_dclk_rate var declaration at the top of the function can of course
go away.

That way we still keep documenting how these values came to be:

/*
 * if_dclk_rate = dclk_core_rate / K;
 * *if_pixclk_div = dclk_rate / if_pixclk_rate;
 * *if_dclk_div = dclk_rate / if_dclk_rate;
 */

Thanks
Heiko





Re: [PATCH] drm/rockchip: vop2: Drop superfluous include

2024-01-04 Thread Heiko Stübner
Am Donnerstag, 4. Januar 2024, 15:39:49 CET schrieb Cristian Ciocaltea:
> The rockchip_drm_fb.h header contains just a single function which is
> not directly used by the VOP2 driver.  Drop the unnecessary include.
> 
> Signed-off-by: Cristian Ciocaltea 

applied to drm-misc-next-fixes as
commit 38709af26c33e398c3292e96837ccfde41fd9e6b


Regards,
Heiko




Re: [PATCH] drm/rockchip: vop2: Drop unused if_dclk_rate variable

2024-01-04 Thread Heiko Stübner
Hi Christian, Andy,

Am Donnerstag, 4. Januar 2024, 15:39:50 CET schrieb Cristian Ciocaltea:
> Commit 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588")
> introduced a variable which ended up being unused.  Remove it.
> 
> rockchip_drm_vop2.c:1688:23: warning: variable ‘if_dclk_rate’ set but not 
> used [-Wunused-but-set-variable]
> 
> Signed-off-by: Cristian Ciocaltea 

in general, please don't send non-series patches as replies to other patches.
It confuses tooling like b4 way too often, as this patch is not designated
as a 2/2 (similar to the first one not being 1/2).

> ---
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 44508c2dd614..923985d4161b 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1685,7 +1685,6 @@ static unsigned long rk3588_calc_cru_cfg(struct 
> vop2_video_port *vp, int id,
>   unsigned long dclk_core_rate = v_pixclk >> 2;
>   unsigned long dclk_rate = v_pixclk;
>   unsigned long dclk_out_rate;
> - unsigned long if_dclk_rate;
>   unsigned long if_pixclk_rate;
>   int K = 1;
>  
> @@ -1700,7 +1699,6 @@ static unsigned long rk3588_calc_cru_cfg(struct 
> vop2_video_port *vp, int id,
>   }
>  
>   if_pixclk_rate = (dclk_core_rate << 1) / K;
> - if_dclk_rate = dclk_core_rate / K;
>   /*
>* *if_pixclk_div = dclk_rate / if_pixclk_rate;
>* *if_dclk_div = dclk_rate / if_dclk_rate;
>*/
*if_pixclk_div = 2;
*if_dclk_div = 4;

with the code continuing with those static constants but the comment
showing a forumula, I do hope Andy can provide a bit of insight into
what is happening here.

I.e. I'd really like to understand if that really is just a remnant or
something different is needed.


Heiko




Re: [PATCH v4 01/29] dt-bindings: display: rockchip, inno-hdmi: Document RK3128 compatible

2023-12-29 Thread Heiko Stübner
Am Freitag, 22. Dezember 2023, 18:41:52 CET schrieb Alex Bee:
> The integration for this SoC is different from the currently existing: It
> needs it's PHY's reference clock rate to calculate the DDC bus frequency
> correctly. The controller is also part of a powerdomain, so this gets added
> as an mandatory property for this variant.
> 
> Signed-off-by: Alex Bee 
> Reviewed-by: Conor Dooley 

this is just a note to myself:

The binding conversion to yaml was picked by Rob in [0], so will most 
likely be part of 6.8-rc1, so this will need to wait until 6.8-rc1 gets into
drm-misc-next .


Heiko


[0] 
https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/commit/?h=for-next&id=7048708fec3a401f9a70e4a74e2e12aa7f88c132

> ---
> changes in v2:
>  - clarify that the controller itself is part of the powerdomain
>  - simplify clock-names
>  - made power-domains property only allowed (and required) for new variant
> 
> changes in v3:
>  - collect RB
> 
> changes in v4:
>  - none
> 
>  .../display/rockchip/rockchip,inno-hdmi.yaml  | 40 ++-
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml 
> b/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml
> index 96889c86849a..be78dcfa1c76 100644
> --- 
> a/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml
> +++ 
> b/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml
> @@ -14,6 +14,7 @@ properties:
>compatible:
>  enum:
>- rockchip,rk3036-inno-hdmi
> +  - rockchip,rk3128-inno-hdmi
>  
>reg:
>  maxItems: 1
> @@ -22,10 +23,19 @@ properties:
>  maxItems: 1
>  
>clocks:
> -maxItems: 1
> +minItems: 1
> +items:
> +  - description: The HDMI controller main clock
> +  - description: The HDMI PHY reference clock
>  
>clock-names:
> -const: pclk
> +minItems: 1
> +items:
> +  - const: pclk
> +  - const: ref
> +
> +  power-domains:
> +maxItems: 1
>  
>ports:
>  $ref: /schemas/graph.yaml#/properties/ports
> @@ -55,6 +65,32 @@ required:
>- pinctrl-names
>- ports
>  
> +allOf:
> +  - if:
> +  properties:
> +compatible:
> +  contains:
> +const: rockchip,rk3036-inno-hdmi
> +
> +then:
> +  properties:
> +power-domains: false
> +
> +  - if:
> +  properties:
> +compatible:
> +  contains:
> +const: rockchip,rk3128-inno-hdmi
> +
> +then:
> +  properties:
> +clocks:
> +  minItems: 2
> +clock-names:
> +  minItems: 2
> +  required:
> +- power-domains
> +
>  additionalProperties: false
>  
>  examples:
> 






Re: [PATCH 5/6] arm64: dts: rockchip: Fix some dtb-check warnings

2023-12-23 Thread Heiko Stübner
Am Freitag, 22. Dezember 2023, 12:05:45 CET schrieb Manuel Traut:
> devicetree checks show some warnings:
> 
> video-codec@fdea0400: 'interrupt-names' is a required property
> from schema $id: http://devicetree.org/schemas/media/rockchip-vpu.yaml#
> 
> hdmi@fe0a: Unevaluated properties are not allowed ('power-domains' were 
> unexpected)
> from schema $id: 
> http://devicetree.org/schemas/display/rockchip/rockchip,dw-hdmi.yaml#
> 
> i2s@fe42: reset-names:0: 'm' is not one of ['tx-m', 'rx-m']
> from schema $id: http://devicetree.org/schemas/sound/rockchip,i2s-tdm.yaml#
> 
> phy@fe87: 'power-domains' is a required property
> from schema $id: 
> http://devicetree.org/schemas/phy/rockchip-inno-csi-dphy.yaml#
> 
> Fix them by
>   - setting a interrupt-name for the video-codec
>   - remove the unevaluated power-domain property from hdmi
>   - set reset-names according to the spec for i2s
>   - add a power-domain property for the CSI phy
> 
> Signed-off-by: Manuel Traut 
> ---
>  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi 
> b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index c19c0f1b3778..651156759582 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -597,6 +597,7 @@ vpu: video-codec@fdea0400 {
>   compatible = "rockchip,rk3568-vpu";
>   reg = <0x0 0xfdea 0x0 0x800>;
>   interrupts = ;
> + interrupt-names = "vdpu";
>   clocks = <&cru ACLK_VPU>, <&cru HCLK_VPU>;
>   clock-names = "aclk", "hclk";
>   iommus = <&vdpu_mmu>;
> @@ -819,7 +820,6 @@ hdmi: hdmi@fe0a {
>   clock-names = "iahb", "isfr", "cec", "ref";
>   pinctrl-names = "default";
>   pinctrl-0 = <&hdmitx_scl &hdmitx_sda &hdmitxm0_cec>;
> - power-domains = <&power RK3568_PD_VO>;

are you really sure that the hdmi controller is _not_ part of
the VO powerdomain? I.e. Depending on that knowledge it could
also simply be necessary to add the property to the binding.


Heiko




Re: [PATCH v4 15/17] dt-bindings: iommu: rockchip: Add Rockchip RK3588

2023-12-09 Thread Heiko Stübner
Hi Andy,

Am Samstag, 9. Dezember 2023, 02:26:25 CET schrieb Andy Yan:
> Hi Heiko:
> 
> On 12/9/23 00:29, Heiko Stübner wrote:
> > Am Donnerstag, 7. Dezember 2023, 09:02:35 CET schrieb Andy Yan:
> >> From: Andy Yan 
> >>
> >> Add a Rockchip RK3588 compatible
> >>
> >> Signed-off-by: Andy Yan 
> > 
> > Reviewed-by: Heiko Stuebner 
> > 
> > In the next iteration, please split this out into a separate patch and send
> > it to the iommu+dt maintainers.
> > 
> > Supporting the iommus on rk3588 can be realized separately and the
> > patch needs to go through a separate tree anyway.
> 
> Okay, I will split this patch out of this vop2 series, does this mean that
> I also need to split out the iommu dt node as a separate patch from current
> PATCH16 ?

no :-) .

The vop-iommu can stay together with the vop core node.
As adding the actual vop iommu is very much tied to the vop node.

For the iommu-compatible it was different. The compatible just declares
that the iommu acts similar to the one on rk3568, so is related to the iommu
driver itself and should go through the iommu tree.

So all is good here.

Heiko




Re: [PATCH v4 15/17] dt-bindings: iommu: rockchip: Add Rockchip RK3588

2023-12-08 Thread Heiko Stübner
Am Donnerstag, 7. Dezember 2023, 09:02:35 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> Add a Rockchip RK3588 compatible
> 
> Signed-off-by: Andy Yan 

Reviewed-by: Heiko Stuebner 

In the next iteration, please split this out into a separate patch and send
it to the iommu+dt maintainers.

Supporting the iommus on rk3588 can be realized separately and the
patch needs to go through a separate tree anyway.


Thanks
Heiko





Re: [PATCH v4 16/17] arm64: dts: rockchip: Add vop on rk3588

2023-12-08 Thread Heiko Stübner
Hi Andy,

Am Donnerstag, 7. Dezember 2023, 09:02:47 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> Add vop dt node for rk3588.
> 
> Signed-off-by: Andy Yan 
> ---
> 
> (no changes since v1)
> 
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 96 +++
>  1 file changed, 96 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi 
> b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 7064c0e9179f..a9810ca78dc4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -593,6 +608,87 @@ i2c0: i2c@fd88 {
>   status = "disabled";
>   };
>  
> + vop: vop@fdd9 {
> + compatible = "rockchip,rk3588-vop";
> + reg = <0x0 0xfdd9 0x0 0x4200>, <0x0 0xfdd95000 0x0 0x1000>;
> + reg-names = "vop", "gamma_lut";
> + interrupts = ;
> + clocks = <&cru ACLK_VOP>,
> +  <&cru HCLK_VOP>,
> +  <&cru DCLK_VOP0>,
> +  <&cru DCLK_VOP1>,
> +  <&cru DCLK_VOP2>,
> +  <&cru DCLK_VOP3>,
> +  <&cru PCLK_VOP_ROOT>;
> + clock-names = "aclk",
> +   "hclk",
> +   "dclk_vp0",
> +   "dclk_vp1",
> +   "dclk_vp2",
> +   "dclk_vp3",
> +   "pclk_vop";
> + resets = <&cru SRST_A_VOP>,
> +  <&cru SRST_H_VOP>,
> +  <&cru SRST_D_VOP0>,
> +  <&cru SRST_D_VOP1>,
> +  <&cru SRST_D_VOP2>,
> +  <&cru SRST_D_VOP3>;
> + reset-names = "axi",
> +   "ahb",
> +   "dclk_vp0",
> +   "dclk_vp1",
> +   "dclk_vp2",
> +   "dclk_vp3";

resets and reset-names do not seem to be part of the binding, so
should probably be added there.


> + iommus = <&vop_mmu>;
> + power-domains = <&power RK3588_PD_VOP>;
> + rockchip,grf = <&sys_grf>;
> + rockchip,vop-grf = <&vop_grf>;
> + rockchip,vo1-grf = <&vo1_grf>;
> + rockchip,pmu = <&pmu>;
> +

move this blank line _below_ the status=disabled please.

> + status = "disabled";
> + vop_out: ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + vp0: port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> + };
> +
> + vp1: port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + };
> +
> + vp2: port@2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <2>;
> + };
> +
> + vp3: port@3 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <3>;
> + };
> + };
> + };
> +
> + vop_mmu: iommu@fdd97e00 {
> + compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + reg = <0x0 0xfdd97e00 0x0 0x100>, <0x0 0xfdd97f00 0x0 0x100>;
> + interrupts = ;
> + interrupt-names = "vop_mmu";

interrupt-names is not part of the mainline iommu binding, so should be dropped


Thanks
Heiko




Re: [PATCH v4 00/17] Add VOP2 support on rk3588

2023-12-08 Thread Heiko Stübner
Hi Andy,

Am Donnerstag, 7. Dezember 2023, 08:59:06 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> This patch sets aims at enable the VOP2 support on rk3588.
> 
> Main feature of VOP2 on rk3588:
> Four video ports:
> VP0 Max 4096x2160
> VP1 Max 4096x2160
> VP2 Max 4096x2160
> VP3 Max 2048x1080
> 
> 4 4K Cluster windows with AFBC/line RGB and AFBC-only YUV support
> 4 4K Esmart windows with line RGB/YUV support
> 
> The current version support all the 8 windows with all the suppported
> plane format.
> 
> And we don't have a upstreamed encoder/connector(HDMI/DP) for rk3588
> yet, Cristian from collabora is working on adding upstream support for
> HDMI on rk3588.
> 
> My current test(1080P/4KP60) is runing with a HDMI driver pick from
> downstream bsp kernel.
> 
> A branch based on linux-6.7 rc4 containing all the series and
> HDMI driver(not compatible with mainline rk3568 hdmi) picked
> from downstream bsp kernel is available [0].
> 
> [0]https://github.com/andyshrk/linux/commits/rk3588-vop2-upstream-linux-6.7-rc4-2023-12-07
> 
> Changes in v4:
> - fix the POST_BUF_EMPTY irq when set mode
> - use full stop at all the description's end.
> - address Krzysztof's review about dt-binding in v3
> - keep all VOP2_FEATURE_HAS_xxx macros increase in order.
> - address Sascha's review about debugfs
> - Add const for rockchip,rk3588-iommu compatible

very nice, the error messages on "mode changes" are gone now.
Display and even combination with panthor still work of my rk3588-board

Tested-by: Heiko Stuebner 






Re: [PATCH V2 00/10] rockchip: Add Powkiddy X55

2023-12-05 Thread Heiko Stübner
Am Dienstag, 5. Dezember 2023, 09:28:24 CET schrieb Neil Armstrong:
> On 05/12/2023 09:26, Neil Armstrong wrote:
> > Hi,
> > 
> > On Mon, 04 Dec 2023 12:57:09 -0600, Chris Morgan wrote:
> >> From: Chris Morgan 
> >>
> >> Add support for the Rockchip RK3566 based Powkiddy X55 handheld gaming
> >> console.
> >>
> >> Changes since V1:
> >>   - Corrected a bug with the DRM mode flags for the video driver.
> >>   - Adjusted panel front and back porch and pixel clock to fix
> >> issues with display that occurred after correcting DRM mode
> >> flag bug.
> >>   - Add a new clk frequency for PLL_VPLL to get panel to run at ~60hz.
> >>
> >> [...]
> > 
> > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git 
> > (drm-misc-next)
> > 
> > [01/10] drm/panel: himax-hx8394: Drop prepare/unprepare tracking
> >  
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8c2c5d1d33f0725b7995f44f87a81311d13a441d
> > [02/10] drm/panel: himax-hx8394: Drop shutdown logic
> >  
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e4f53a4d921eba6187a2599cf184a3beeb604fe2
> > [03/10] dt-bindings: display: Document Himax HX8394 panel rotation
> >  
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=be478bc7ab08127473ce9ed893378cc2a8762611
> > [04/10] drm/panel: himax-hx8394: Add Panel Rotation Support
> >  
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a695a5009c8fd239a98d98209489997ff5397d2b
> > [05/10] dt-bindings: display: himax-hx8394: Add Powkiddy X55 panel
> >  
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=00830a0d8f0d820335e7beb26e251069d90f2574
> > [06/10] drm/panel: himax-hx8394: Add Support for Powkiddy X55 panel
> >  
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=38db985966d2f0f89f7e1891253489a16936fc5e
> > [07/10] clk: rockchip: Mark pclk_usb as critical on rk3568
> >  (no commit info)
> > [08/10] clk: rockchip: rk3568: Add PLL rate for 126.4MHz
> >  (no commit info)
> > [09/10] dt-bindings: arm: rockchip: Add Powkiddy X55
> >  (no commit info)
> > [10/10] arm64: dts: rockchip: Add Powkiddy X55
> >  (no commit info)
> > 
> 
> To clarify, only patches 1 to 6 were applied to drm-misc-next,

thanks for the clarification (and applying the patches already)

Heiko




Re: [PATCH v2 2/5] ARM: dts: rockchip: Add power-controller for RK3128

2023-12-03 Thread Heiko Stübner
Hi Alex,

Am Sonntag, 3. Dezember 2023, 17:05:47 CET schrieb Alex Bee:
> Am 02.12.23 um 18:46 schrieb Heiko Stübner:
> > Am Samstag, 2. Dezember 2023, 17:36:15 CET schrieb Alex Bee:
> >> Am 02.12.23 um 16:51 schrieb Heiko Stübner:
> >>> Am Samstag, 2. Dezember 2023, 13:51:41 CET schrieb Alex Bee:
> >>>> Add power controller and qos nodes for RK3128 in order to use
> >>>> them as powerdomains.
> >>> does the power-domain controller work with the incomplete set of
> >>> pm-domains too?
> >> Yes, it does - the missing domains can request idle only and can't be
> >> powered on/off - if no one requests idle they are just up all the time.
> >>
> >>> What I have in mind is
> >>> - adding the power-controller node with the existing set of power-domains
> >>> - the gpu pm-domain is in there
> >>> - adding the gpu parts
> >> My main concern about adding them later was the change of the ABI after
> >> they've been exposed in the SoC DT. If that's not an issue - sure: I can
> >> add them in a separate series.
> > An ABI change would be _changing_ the domain-ids in the rk3128-power.h
> > I think :-) .
> Well, an addition is still a change.
> > Right now the existing domain ids in the header are already exposed to the
> > world, so someone could already use them, but not the new ones.
> 
> I'm fully aware that nothing would ever hard fail anywhere if the new 
> domain ids get added later.
> 
> Nevertheless we start using here an ABI which is known to be incomplete. 
> For no reason, as the patches (which I am now asked to remove from this 
> series) for completion are already there (here).
> 
> Anyway, if you prefer it this way: I'm pleased to do so.

I was more thinking of accelerating the gpu-part of the series, as that
really is just waiting for the power-domain node that already has driver
support and domain-ids present.

It looks like you're feeling more strongly about that though, so I'll
definitly not pressure you ;-) .

But I guess the split into IDs and driver change should still be
done, especially as the dt-binding-header likely will want an Ack
from the DT maintainers.

And the power-domain change will go through the new pmdomain
subsystem.


Heiko


> >>> And a second series with
> >>> - patch1 from here
> >>> - a dts patch adding the additional pm-domains to rk3128.dtsi
> >>> - I guess patch1 also should be split into a patch adding the binding-ids
> >>> and a separate patch for the code addition.
> >> Yeah, I noticed this also :)
> >>
> >> Regards,
> >>
> >> Alex
> >>
> >>>
> >>> Heiko
> >>>
> >>>> Signed-off-by: Alex Bee 
> >>>> ---
> >>>>arch/arm/boot/dts/rockchip/rk3128.dtsi | 101 +
> >>>>1 file changed, 101 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm/boot/dts/rockchip/rk3128.dtsi 
> >>>> b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> >>>> index 4e8b38604ecd..b72905db04f7 100644
> >>>> --- a/arch/arm/boot/dts/rockchip/rk3128.dtsi
> >>>> +++ b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> >>>> @@ -8,6 +8,7 @@
> >>>>#include 
> >>>>#include 
> >>>>#include 
> >>>> +#include 
> >>>>
> >>>>/ {
> >>>>  compatible = "rockchip,rk3128";
> >>>> @@ -133,6 +134,106 @@ smp-sram@0 {
> >>>>  pmu: syscon@100a {
> >>>>  compatible = "rockchip,rk3128-pmu", "syscon", 
> >>>> "simple-mfd";
> >>>>  reg = <0x100a 0x1000>;
> >>>> +
> >>>> +power: power-controller {
> >>>> +compatible = "rockchip,rk3128-power-controller";
> >>>> +#power-domain-cells = <1>;
> >>>> +#address-cells = <1>;
> >>>> +#size-cells = <0>;
> >>>> +
> >>>> +power-domain@RK3128_PD_VIO {
> >>>> +reg = ;
> >>>> +clocks = <&cru ACLK_CIF>,
> >>>> + <&cru HCLK_CIF>,
> >>>> +   

Re: [PATCH v2 2/5] ARM: dts: rockchip: Add power-controller for RK3128

2023-12-02 Thread Heiko Stübner
Hi Alex,

Am Samstag, 2. Dezember 2023, 17:36:15 CET schrieb Alex Bee:
> Am 02.12.23 um 16:51 schrieb Heiko Stübner:
> > Am Samstag, 2. Dezember 2023, 13:51:41 CET schrieb Alex Bee:
> >> Add power controller and qos nodes for RK3128 in order to use
> >> them as powerdomains.
> > does the power-domain controller work with the incomplete set of
> > pm-domains too?
> 
> Yes, it does - the missing domains can request idle only and can't be 
> powered on/off - if no one requests idle they are just up all the time.
> 
> > What I have in mind is
> > - adding the power-controller node with the existing set of power-domains
> > - the gpu pm-domain is in there
> > - adding the gpu parts
> 
> My main concern about adding them later was the change of the ABI after 
> they've been exposed in the SoC DT. If that's not an issue - sure: I can 
> add them in a separate series.

An ABI change would be _changing_ the domain-ids in the rk3128-power.h
I think :-) .

Right now the existing domain ids in the header are already exposed to the
world, so someone could already use them, but not the new ones.



Heiko

> > And a second series with
> > - patch1 from here
> > - a dts patch adding the additional pm-domains to rk3128.dtsi
> > - I guess patch1 also should be split into a patch adding the binding-ids
> >and a separate patch for the code addition.
> 
> Yeah, I noticed this also :)
> 
> Regards,
> 
> Alex
> 
> >
> >
> > Heiko
> >
> >> Signed-off-by: Alex Bee 
> >> ---
> >>   arch/arm/boot/dts/rockchip/rk3128.dtsi | 101 +
> >>   1 file changed, 101 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/rockchip/rk3128.dtsi 
> >> b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> >> index 4e8b38604ecd..b72905db04f7 100644
> >> --- a/arch/arm/boot/dts/rockchip/rk3128.dtsi
> >> +++ b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> >> @@ -8,6 +8,7 @@
> >>   #include 
> >>   #include 
> >>   #include 
> >> +#include 
> >>   
> >>   / {
> >>compatible = "rockchip,rk3128";
> >> @@ -133,6 +134,106 @@ smp-sram@0 {
> >>pmu: syscon@100a {
> >>compatible = "rockchip,rk3128-pmu", "syscon", "simple-mfd";
> >>reg = <0x100a 0x1000>;
> >> +
> >> +  power: power-controller {
> >> +  compatible = "rockchip,rk3128-power-controller";
> >> +  #power-domain-cells = <1>;
> >> +  #address-cells = <1>;
> >> +  #size-cells = <0>;
> >> +
> >> +  power-domain@RK3128_PD_VIO {
> >> +  reg = ;
> >> +  clocks = <&cru ACLK_CIF>,
> >> +   <&cru HCLK_CIF>,
> >> +   <&cru DCLK_EBC>,
> >> +   <&cru HCLK_EBC>,
> >> +   <&cru ACLK_IEP>,
> >> +   <&cru HCLK_IEP>,
> >> +   <&cru ACLK_LCDC0>,
> >> +   <&cru HCLK_LCDC0>,
> >> +   <&cru PCLK_MIPI>,
> >> +   <&cru ACLK_RGA>,
> >> +   <&cru HCLK_RGA>,
> >> +   <&cru ACLK_VIO0>,
> >> +   <&cru ACLK_VIO1>,
> >> +   <&cru HCLK_VIO>,
> >> +   <&cru HCLK_VIO_H2P>,
> >> +   <&cru DCLK_VOP>,
> >> +   <&cru SCLK_VOP>;
> >> +  pm_qos = <&qos_ebc>,
> >> +   <&qos_iep>,
> >> +   <&qos_lcdc>,
> >> +   <&qos_rga>,
> >> +   <&qos_vip>;
> >> +  #power-domain-cells = <0>;
> >> +  };
> >> +
> >> +  power-domain@RK3128_PD_VIDEO {
> >

Re: [PATCH v2 2/5] ARM: dts: rockchip: Add power-controller for RK3128

2023-12-02 Thread Heiko Stübner
Hi Alex,

Am Samstag, 2. Dezember 2023, 13:51:41 CET schrieb Alex Bee:
> Add power controller and qos nodes for RK3128 in order to use
> them as powerdomains.

does the power-domain controller work with the incomplete set of
pm-domains too?

What I have in mind is
- adding the power-controller node with the existing set of power-domains
- the gpu pm-domain is in there
- adding the gpu parts


And a second series with
- patch1 from here
- a dts patch adding the additional pm-domains to rk3128.dtsi
- I guess patch1 also should be split into a patch adding the binding-ids
  and a separate patch for the code addition.


Heiko

> Signed-off-by: Alex Bee 
> ---
>  arch/arm/boot/dts/rockchip/rk3128.dtsi | 101 +
>  1 file changed, 101 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/rockchip/rk3128.dtsi 
> b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> index 4e8b38604ecd..b72905db04f7 100644
> --- a/arch/arm/boot/dts/rockchip/rk3128.dtsi
> +++ b/arch/arm/boot/dts/rockchip/rk3128.dtsi
> @@ -8,6 +8,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  / {
>   compatible = "rockchip,rk3128";
> @@ -133,6 +134,106 @@ smp-sram@0 {
>   pmu: syscon@100a {
>   compatible = "rockchip,rk3128-pmu", "syscon", "simple-mfd";
>   reg = <0x100a 0x1000>;
> +
> + power: power-controller {
> + compatible = "rockchip,rk3128-power-controller";
> + #power-domain-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + power-domain@RK3128_PD_VIO {
> + reg = ;
> + clocks = <&cru ACLK_CIF>,
> +  <&cru HCLK_CIF>,
> +  <&cru DCLK_EBC>,
> +  <&cru HCLK_EBC>,
> +  <&cru ACLK_IEP>,
> +  <&cru HCLK_IEP>,
> +  <&cru ACLK_LCDC0>,
> +  <&cru HCLK_LCDC0>,
> +  <&cru PCLK_MIPI>,
> +  <&cru ACLK_RGA>,
> +  <&cru HCLK_RGA>,
> +  <&cru ACLK_VIO0>,
> +  <&cru ACLK_VIO1>,
> +  <&cru HCLK_VIO>,
> +  <&cru HCLK_VIO_H2P>,
> +  <&cru DCLK_VOP>,
> +  <&cru SCLK_VOP>;
> + pm_qos = <&qos_ebc>,
> +  <&qos_iep>,
> +  <&qos_lcdc>,
> +  <&qos_rga>,
> +  <&qos_vip>;
> + #power-domain-cells = <0>;
> + };
> +
> + power-domain@RK3128_PD_VIDEO {
> + reg = ;
> + clocks = <&cru ACLK_VDPU>,
> +  <&cru HCLK_VDPU>,
> +  <&cru ACLK_VEPU>,
> +  <&cru HCLK_VEPU>,
> +  <&cru SCLK_HEVC_CORE>;
> + pm_qos = <&qos_vpu>;
> + #power-domain-cells = <0>;
> + };
> +
> + power-domain@RK3128_PD_GPU {
> + reg = ;
> + clocks = <&cru ACLK_GPU>;
> + pm_qos = <&qos_gpu>;
> + #power-domain-cells = <0>;
> + };
> +
> + power-domain@RK3128_PD_CRYPTO {
> + reg = ;
> + clocks = <&cru HCLK_CRYPTO>,
> +  <&cru SCLK_CRYPTO>;
> + pm_qos = <&qos_crypto>;
> + #power-domain-cells = <0>;
> + };
> + };
> + };
> +
> + qos_crypto: qos@10128080 {
> + compatible = "rockchip,rk3128-qos", "syscon";
> + reg = <0x10128080 0x20>;
> + };
> +
> + qos_gpu: qos@1012d000 {
> + compatible = "rockchip,rk3128-qos", "syscon";
> + reg = <0x1012d000 0x20>;
> + };
> +
> + qos_vpu: qos@1012e000 {
> + compatible = "rockchip,rk3128-qos", "syscon";
> + reg = <0x1012e000 0x20>;
> + };
> +
> + qos_rga: qos@1012f000 {
> + compatible = "rockchip,rk3128-qos", "syscon";
> + reg = <0x1012f000 0x20>;
> + };
> +
> + qos_ebc: qos@1012f080 {
> + compatible = "rockchip,rk3128-qos", "syscon";
> + reg = <0x1012f080 0x20>;
> +   

Re: [PATCH v3 13/14] dt-bindings: iommu: rockchip: Add Rockchip RK3588

2023-11-30 Thread Heiko Stübner
Hi Andy,

Am Donnerstag, 30. November 2023, 13:25:00 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> Add a Rockchip RK3588 compatible
> 
> Signed-off-by: Andy Yan 
> ---
> 
> (no changes since v1)
> 
>  Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml 
> b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> index ba9124f721f1..3febf0c3c404 100644
> --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> @@ -22,6 +22,7 @@ properties:
>  enum:
>- rockchip,iommu
>- rockchip,rk3568-iommu
> +  - rockchip,rk3588-iommu

This enum only allows that the compatible has one element, namely one of
those listed here. In the dts though you declare
compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
meaning that the rk3588-iommu is compatible to the rk3568-iommu

I think you'll need a construct like:

properties:
  compatible:
oneOf:
  - enum:
  - rockchip,iommu
  - rockchip,rk3568-iommu
  - items:
  - enum:
  - rockchip,rk3588-iommu
  - const: rockchip,rk3568-iommu

to describe both the single-compatibles and the 2-item compatible for the
rk3588. For example pci/rockchip-dw-pcie.yaml does a similar thing already.

Heiko




Re: [PATCH v2 10/12] drm/rockchip: vop2: Add support for rk3588

2023-11-28 Thread Heiko Stübner
Hi Andy,

Am Dienstag, 28. November 2023, 10:32:55 CET schrieb Andy Yan:
> On 11/27/23 23:29, Heiko Stübner wrote:
> > Am Mittwoch, 22. November 2023, 13:55:44 CET schrieb Andy Yan:
> >> From: Andy Yan 
> >>
> >> VOP2 on rk3588:
> >>
> >> Four video ports:
> >> VP0 Max 4096x2160
> >> VP1 Max 4096x2160
> >> VP2 Max 4096x2160
> >> VP3 Max 2048x1080
> >>
> >> 4 4K Cluster windows with AFBC/line RGB and AFBC-only YUV support
> >> 4 4K Esmart windows with line RGB/YUV support
> >>
> >> Signed-off-by: Andy Yan 
> >>
> >> ---
> >>
> >> Changes in v2:
> >> - add rk3588_ prefix for functions which are rk3588 only
> >> - make some calculation as fixed value and keep calculation formula as
> >>comment
> >> - check return value for some cru calculation functions.
> >> - check return value for syscon_regmap_lookup_by_phandle
> >> - add NV20/NV30 for esmart plane
> >>
> >>   drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 381 ++-
> >>   drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  66 
> >>   drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 221 +++
> >>   3 files changed, 660 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c 
> >> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> index 4bcc405bcf11..9eecbe1f71f9 100644
> >> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> >> @@ -271,9 +282,12 @@ static bool vop2_cluster_window(const struct vop2_win 
> >> *win)
> >>   static void vop2_cfg_done(struct vop2_video_port *vp)
> >>   {
> >>struct vop2 *vop2 = vp->vop2;
> >> +  u32 val;
> >> +
> >> +  val = BIT(vp->id) | (BIT(vp->id) << 16) |
> >> +  RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN;
> >>   
> >> -  regmap_set_bits(vop2->map, RK3568_REG_CFG_DONE,
> >> -  BIT(vp->id) | RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN);
> >> +  regmap_set_bits(vop2->map, RK3568_REG_CFG_DONE, val);
> > I don't fully understand that code:
> > (1) the write mask is also present on the rk3568, so should this change
> >  be a separate patch with a fixes tag?
> 
> The write mask of VP config done on rk356x is missing, that means
> you can write the corresponding mask bit, but it has no effect.
>
> I once considered making it a separate patch,  I can split it as a separate 
> patch if
> you like.

I think I'd like it to be a separate patch please.


> > (2) RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN does not contain the part for
> >  the write-mask
> >
> > #define RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN BIT(15)
> >
> >  why is this working then?
> 
> 
> Actually this bit has no write-mask bit. 🙂

when doing that separate patch mentioned above, could you also add a
comment to the code stating that RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN
doesn't have a write mask bit please?

Because the TRM is not clear and ideally I'd not forget this fact for
the future :-) .


> >>   }
> >>   
> >>   static void vop2_win_disable(struct vop2_win *win)
> > [...]
> >
> >> @@ -1298,7 +1346,11 @@ static void vop2_plane_atomic_update(struct 
> >> drm_plane *plane,
> >>vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1);
> >>vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format);
> >>vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap);
> >> -  vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0);
> >> +  if (vop2->data->soc_id == 3566 || vop2->data->soc_id == 3568)
> >> +  vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0);
> >> +  else
> >> +  vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 1);
> >> +
> > I think this at least warrants a comment, what is happening here. Also,
> > can you already see how future vop2-users are behaving - aka are all new
> > socs in the "else" part of the conditional, or would a switch-case better
> > represent future socs?
> 
> 
> On rk356x, this bit is auto gating enable, but this function is not work well 
> so
> we need to disable this function.
> On rk3588, and the following new soc(rk3528/rk3576), this bit is gating 
> disable,
> we should write 1 to disable gating when enable 

Re: [PATCH v2 04/12] drm/rockchip: vop2: clear afbc en and transform bit for cluster window at linear mode

2023-11-28 Thread Heiko Stübner
Am Dienstag, 28. November 2023, 09:03:46 CET schrieb Andy Yan:
> Hi Heiko:
> 
> On 11/27/23 23:02, Heiko Stübner wrote:
> > Am Mittwoch, 22. November 2023, 13:54:25 CET schrieb Andy Yan:
> >> From: Andy Yan 
> >>
> >> The enable bit and transform offset of cluster windows should be
> >> cleared when it work at linear mode, or we may have a iommu fault
> >> issue.
> >>
> >> Signed-off-by: Andy Yan 
> > I guess same here?
> >
> > Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")
> 
> 
> I'm not sure if we need a Fixes tag here,  in fact this issue never happens on
> 
> rk3566/8 , because the cluster windows of rk356x only support afbc format,
> 
> they don't have a chance to switch between afbc and linear mode.
> 
> Of course, the lack support of linear mode of rk356x cluster windows is a 
> thoughtless
> 
> of IC design, if it really support both afbc and linear format, we indeed 
> need this fix.
> 
> The situation is the same as patch 03/12.
> 
> So I hope follow your advice, if it need a Fixes tag here.

ah ok, thanks for the explanation. Then I guess we don't need a fixes tag
when the rk3568 is not affected by this.

Same for the other patch. If you're re-sending you could add this information
to the commit message though. (existing support for rk3568 only supports
afbc cluster windows and is therefore not affected)


Thanks
Heiko




Re: [PATCH v2 10/12] drm/rockchip: vop2: Add support for rk3588

2023-11-27 Thread Heiko Stübner
Hi Andy,

Am Mittwoch, 22. November 2023, 13:55:44 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> VOP2 on rk3588:
> 
> Four video ports:
> VP0 Max 4096x2160
> VP1 Max 4096x2160
> VP2 Max 4096x2160
> VP3 Max 2048x1080
> 
> 4 4K Cluster windows with AFBC/line RGB and AFBC-only YUV support
> 4 4K Esmart windows with line RGB/YUV support
> 
> Signed-off-by: Andy Yan 
> 
> ---
> 
> Changes in v2:
> - add rk3588_ prefix for functions which are rk3588 only
> - make some calculation as fixed value and keep calculation formula as
>   comment
> - check return value for some cru calculation functions.
> - check return value for syscon_regmap_lookup_by_phandle
> - add NV20/NV30 for esmart plane
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 381 ++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.h |  66 
>  drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 221 +++
>  3 files changed, 660 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 4bcc405bcf11..9eecbe1f71f9 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

> @@ -271,9 +282,12 @@ static bool vop2_cluster_window(const struct vop2_win 
> *win)
>  static void vop2_cfg_done(struct vop2_video_port *vp)
>  {
>   struct vop2 *vop2 = vp->vop2;
> + u32 val;
> +
> + val = BIT(vp->id) | (BIT(vp->id) << 16) |
> + RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN;
>  
> - regmap_set_bits(vop2->map, RK3568_REG_CFG_DONE,
> - BIT(vp->id) | RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN);
> + regmap_set_bits(vop2->map, RK3568_REG_CFG_DONE, val);

I don't fully understand that code:
(1) the write mask is also present on the rk3568, so should this change
be a separate patch with a fixes tag?
(2) RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN does not contain the part for
the write-mask

#define RK3568_REG_CFG_DONE__GLB_CFG_DONE_EN BIT(15)

why is this working then?

>  }
>  
>  static void vop2_win_disable(struct vop2_win *win)

[...]

> @@ -1298,7 +1346,11 @@ static void vop2_plane_atomic_update(struct drm_plane 
> *plane,
>   vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1);
>   vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format);
>   vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap);
> - vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0);
> + if (vop2->data->soc_id == 3566 || vop2->data->soc_id == 3568)
> + vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0);
> + else
> + vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 1);
> +

I think this at least warrants a comment, what is happening here. Also,
can you already see how future vop2-users are behaving - aka are all new
socs in the "else" part of the conditional, or would a switch-case better
represent future socs?


>   vop2_win_write(win, VOP2_WIN_AFBC_BLOCK_SPLIT_EN, 0);
>   transform_offset = vop2_afbc_transform_offset(pstate, 
> half_block_en);
>   vop2_win_write(win, VOP2_WIN_AFBC_HDR_PTR, yrgb_mst);


> @@ -1627,9 +1937,17 @@ static void vop2_crtc_atomic_enable(struct drm_crtc 
> *crtc,
>   drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) 
> {
>   struct rockchip_encoder *rkencoder = 
> to_rockchip_encoder(encoder);
>  
> - rk3568_set_intf_mux(vp, rkencoder->crtc_endpoint_id, polflags);
> + /*
> +  * for drive a high resolution(4KP120, 8K), vop on 
> rk3588/rk3576 need
> +  * process multi(1/2/4/8) pixels per cycle, so the dclk feed by 
> the
> +  * system cru may be the 1/2 or 1/4 of mode->clock.
> +  */
> + clock = vop2_set_intf_mux(vp, rkencoder->crtc_endpoint_id, 
> polflags);
>   }
>  
> + if (!clock)
> + return;
> +

hmm, shouldn't the check for the validity of a mode happen before
atomic_enable is run? So this shouldn't error out in the middle of the
function?


>   if (vcstate->output_mode == ROCKCHIP_OUT_MODE_ &&
>   !(vp_data->feature & VOP_FEATURE_OUTPUT_10BIT))
>   out_mode = ROCKCHIP_OUT_MODE_P888;


Thanks
Heiko




Re: [PATCH v2 04/12] drm/rockchip: vop2: clear afbc en and transform bit for cluster window at linear mode

2023-11-27 Thread Heiko Stübner
Am Mittwoch, 22. November 2023, 13:54:25 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> The enable bit and transform offset of cluster windows should be
> cleared when it work at linear mode, or we may have a iommu fault
> issue.
> 
> Signed-off-by: Andy Yan 

I guess same here?

Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")





Re: [PATCH v2 03/12] drm/rockchip: vop2: set half_block_en bit in all mode

2023-11-27 Thread Heiko Stübner
Am Mittwoch, 22. November 2023, 13:54:13 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> At first we thought the half_block_en bit in AFBCD_CTRL register
> only work in afbc mode. But the fact is that it control the line
> buffer in all mode(afbc/tile/line), so we need configure it in
> all case.
> 
> Signed-off-by: Andy Yan 

This looks common to the rk3568 variant, right, so I guess this should
have a

Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver")

perhaps?


Heiko




Re: [PATCH v1 3/4] drm/rockchip: rk3066_hdmi: Remove useless output format

2023-11-27 Thread Heiko Stübner
Hi Johan,

Am Donnerstag, 23. November 2023, 13:54:28 CET schrieb Johan Jonker:
> 
> On 11/20/23 18:06, Heiko Stuebner wrote:
> > Hi Johan,
> > 
> > Am Donnerstag, 2. November 2023, 14:42:19 CET schrieb Johan Jonker:
> >> The Rk3066 hdmi output format is hard coded to RGB. Remove
> >> all useless code related to colorimetry and enc_out_format.
> >>
> >> Signed-off-by: Johan Jonker 
> > 
> 
> > I guess my first question is, is the hardcoding happening just because
> > of missing functionality in the driver, or does the hardware only
> > support RGB?
> 
> This driver can do so much more..., but is crippled by various causes.
> If in need for a full functional rk3066 driver a little bit 
> help/advise/action from other people is needed.

Part of me wants to have fully working drivers, but on the other hand,
both the rk3066-hdmi and also the inno-hdmi drivers are sort of one-off
drivers used by rk3066 and rk3036 (inno-hdmi) and most likely won't see
new SoCs using them in the future.


So I guess after thinking more about this, I should probably just apply
your patch to simplify the code and if by some magical happenings in
future someone really wants to spend time on either one of these drivers
they can always use "git revert" to bring back the old code?


Heiko


> 1:
> Missing rk3066 TRM HDMI register info.
> Could Rockchip (= Sandy Huang) disclose this info to the open source 
> community?
> 
> As a way around we can look at older driver code and port to mainline.
> More info gives better results.
> rk30_hdmi_config_csc() function:
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/chips/rkpx2/rkpx2_hdmi_hw.c#L315
> 
> 2:
> Could DRM people show us examples for:
> - How to advertise to the VOP driver what data formats (RGB, YCBCR) it can 
> send to the HDMI driver or any other Rockchip DRM sub driver other then RGB.
> - Advertise EDID data monitor modes RGB444, YCBCR444 and YCBCR422 to the HDMI 
> driver.
> 
> https://github.com/RockchipOpensourceCommunity/px2-android-kernel-3.0/blob/master/drivers/video/rockchip/hdmi/rk_hdmi_edid.c#L217C1-L218C41
> 
> 3:
> Advise when what Infoframe is needed for only RGB vs. the rest according to 
> the specification.
> https://engineering.purdue.edu/ece477/Archive/2012/Spring/S12-Grp10/Datasheets/CEC_HDMI_Specification.pdf
> 
> rk3066 currently only sends avi info. Does it need vsi as well? Can anyone 
> give some clarity here?
> inno_hdime sends avi and vsi info.
> 
> 4:
> rk3066_hdmi and inno_hdmi are HDMI 1.4a drivers for DVI and HDMI.
> Validated by drm_match_cea_mode() this function only gives us both HDMI + 
> HDMI2 focus, but nothing for old DVI monitors.
> How to improve?
> 
> 5:
> Sound support was submitted:
> Re: [PATCH v6 2/5] drm: rockchip: add sound support to rk3066 hdmi driver
> https://lore.kernel.org/linux-rockchip/48dbe9b7-0aa0-f459-301f-f380e2b7f...@gmail.com/
> 
> No reply was given (by Heiko or others) on why it wasn't applied or what 
> needs to be improved.
> 
> Without reply no improvement.
> 
> Johan
> 
> 
> > 
> > 
> >> ---
> >>  drivers/gpu/drm/rockchip/rk3066_hdmi.c | 20 +---
> >>  1 file changed, 1 insertion(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c 
> >> b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> >> index 0e7aae341960..f2b1b2faa096 100644
> >> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> >> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> >> @@ -23,8 +23,6 @@
> >>
> >>  struct hdmi_data_info {
> >>int vic; /* The CEA Video ID (VIC) of the current drm display mode. */
> >> -  unsigned int enc_out_format;
> >> -  unsigned int colorimetry;
> >>  };
> >>
> >>  struct rk3066_hdmi_i2c {
> >> @@ -200,14 +198,7 @@ static int rk3066_hdmi_config_avi(struct rk3066_hdmi 
> >> *hdmi,
> >>rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> >>  &hdmi->connector, mode);
> >>
> >> -  if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
> >> -  frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> >> -  else if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV422)
> >> -  frame.avi.colorspace = HDMI_COLORSPACE_YUV422;
> >> -  else
> >> -  frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> >> -
> >> -  frame.avi.colorimetry = hdmi->hdmi_data.colorimetry;
> >> +  frame.avi.colorspace = HDMI_COLORSPACE_RGB;
> >>frame.avi.scan_mode = HDMI_SCAN_MODE_NONE;
> >>
> >>return rk3066_hdmi_upload_frame(hdmi, rc, &frame,
> >> @@ -329,15 +320,6 @@ static int rk3066_hdmi_setup(struct rk3066_hdmi *hdmi,
> >>struct drm_display_info *display = &hdmi->connector.display_info;
> >>
> >>hdmi->hdmi_data.vic = drm_match_cea_mode(mode);
> >> -  hdmi->hdmi_data.enc_out_format = HDMI_COLORSPACE_RGB;
> >> -
> >> -  if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 ||
> >> -  hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22 ||
> >> -  hdmi->hdm

Re: [PATCH 09/11] drm/rockchip: vop2: Add support for rk3588

2023-11-15 Thread Heiko Stübner
Am Mittwoch, 15. November 2023, 03:02:42 CET schrieb Andy Yan:
> Hi Heiko:
> 
> On 11/15/23 07:34, Heiko Stübner wrote:
> > Hi Andy,
> >
> > Am Dienstag, 14. November 2023, 12:28:55 CET schrieb Andy Yan:
> >> From: Andy Yan 
> >>
> >> VOP2 on rk3588:
> >>
> >> Four video ports:
> >> VP0 Max 4096x2160
> >> VP1 Max 4096x2160
> >> VP2 Max 4096x2160
> >> VP3 Max 2048x1080
> >>
> >> 4 4K Cluster windows with AFBC/line RGB and AFBC-only YUV support
> >> 4 4K Esmart windows with line RGB/YUV support
> >>
> >> Signed-off-by: Andy Yan 
> > not a review yet, but when testing and the display sets a mode,
> > I always get a bunch of
> >
> > rockchip-drm display-subsystem: [drm] *ERROR* POST_BUF_EMPTY irq err at 
> > vp0
> >
> > messages in the log (initial mode to console, starting glmark2 from console,
> > stopping glmark2 to the console).
> >
> > I'm not sure what is up with that, have you seen these messages as well
> > at some point?
> 
> Yes, it will raise POST_BUF_EMPTY when set a mode,  it needs some fix 
> like [0]:
> 
> 
> I still trying to find a appropriate way to do it with the upstream 
> code, as it doesn't affect the
> 
> real display function(I must admit that the POST_BUF_EMPTY irq is very 
> annoying), so l let  it as
> 
> it is in the current version.

okay, so this is a known thing. So no worries and I'm confident that
you'll figure out a way to do this :-)


> By the way, can you see the glmark2 rending on your HDMI monitor now?

Yes :-D . I do have glmark2 (both es2 and full-gl) running with Boris'
panthor patches [0] merged into my dev-kernel and mesa build
from one his branches [1] .

Heiko


[0] https://gitlab.freedesktop.org/bbrezillon/linux/-/tree/panthor-v3
[1] 
https://gitlab.freedesktop.org/bbrezillon/mesa/-/tree/v10+panthor-v3+32b+g310+egl15+afrc+afbc?ref_type=heads




Re: [PATCH 09/11] drm/rockchip: vop2: Add support for rk3588

2023-11-14 Thread Heiko Stübner
Hi Andy,

Am Dienstag, 14. November 2023, 12:28:55 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> VOP2 on rk3588:
> 
> Four video ports:
> VP0 Max 4096x2160
> VP1 Max 4096x2160
> VP2 Max 4096x2160
> VP3 Max 2048x1080
> 
> 4 4K Cluster windows with AFBC/line RGB and AFBC-only YUV support
> 4 4K Esmart windows with line RGB/YUV support
> 
> Signed-off-by: Andy Yan 

not a review yet, but when testing and the display sets a mode,
I always get a bunch of

rockchip-drm display-subsystem: [drm] *ERROR* POST_BUF_EMPTY irq err at 
vp0

messages in the log (initial mode to console, starting glmark2 from console,
stopping glmark2 to the console).

I'm not sure what is up with that, have you seen these messages as well
at some point?

Thanks
Heiko





Re: [PATCH 08/11] dt-bindings: display: vop2: Add rk3588 support

2023-11-14 Thread Heiko Stübner
Am Dienstag, 14. November 2023, 12:28:41 CET schrieb Andy Yan:
> From: Andy Yan 
> 
> The vop2 on rk3588 is similar to which on rk356x
> but with 4 video outputs and need to reference
> more grf modules.
> 
> Signed-off-by: Andy Yan 
> ---
> 
>  .../display/rockchip/rockchip-vop2.yaml   | 25 +++
>  1 file changed, 25 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml 
> b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> index b60b90472d42..c333c651da1a 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> @@ -20,6 +20,7 @@ properties:
>  enum:
>- rockchip,rk3566-vop
>- rockchip,rk3568-vop
> +  - rockchip,rk3588-vop
>  
>reg:
>  items:
> @@ -48,6 +49,8 @@ properties:
>- description: Pixel clock for video port 0.
>- description: Pixel clock for video port 1.
>- description: Pixel clock for video port 2.
> +  - description: Pixel clock for video port 4.
> +  - description: Peripheral clock for vop on rk3588.
>  
>clock-names:
>  items:
> @@ -56,12 +59,29 @@ properties:
>- const: dclk_vp0
>- const: dclk_vp1
>- const: dclk_vp2
> +  - const: dclk_vp3
> +  - const: pclk_vop

with the error Rob's bot reported, I guess both clocks and clock-names
need a minItems element to mark these new clocks essentially as optional?


>rockchip,grf:
>  $ref: /schemas/types.yaml#/definitions/phandle
>  description:
>Phandle to GRF regs used for misc control
>  
> +  rockchip,vo-grf:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> +  Phandle to VO GRF regs used for misc control, required for rk3588
> +
> +  rockchip,vop-grf:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> +  Phandle to VOP GRF regs used for misc control, required for rk3588
> +
> +  rockchip,pmu:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> +  Phandle to PMU regs used for misc control, required for rk3588
> +
>ports:
>  $ref: /schemas/graph.yaml#/properties/ports
>  
> @@ -81,6 +101,11 @@ properties:
>  description:
>Output endpoint of VP2
>  
> +  port@3:
> +$ref: /schemas/graph.yaml#/properties/port
> +description:
> +  Output endpoint of VP3
> +
>iommus:
>  maxItems: 1
>  
> 






Re: [PATCH 01/20] drivers/gpu/drm/rockchip: remove I2C_CLASS_DDC support

2023-11-13 Thread Heiko Stübner
Am Montag, 13. November 2023, 12:23:25 CET schrieb Heiner Kallweit:
> After removal of the legacy EEPROM driver and I2C_CLASS_DDC support in
> olpc_dcon there's no i2c client driver left supporting I2C_CLASS_DDC.
> Class-based device auto-detection is a legacy mechanism and shouldn't
> be used in new code. So we can remove this class completely now.
> 
> Preferably this series should be applied via the i2c tree.
> 
> Signed-off-by: Heiner Kallweit 

Acked-by: Heiko Stuebner 

> 
> ---
>  drivers/gpu/drm/rockchip/inno_hdmi.c   |1 -
>  drivers/gpu/drm/rockchip/rk3066_hdmi.c |1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
> b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 6e5b922a1..a7739b27c 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -793,7 +793,6 @@ static struct i2c_adapter *inno_hdmi_i2c_adapter(struct 
> inno_hdmi *hdmi)
>   init_completion(&i2c->cmp);
>  
>   adap = &i2c->adap;
> - adap->class = I2C_CLASS_DDC;
>   adap->owner = THIS_MODULE;
>   adap->dev.parent = hdmi->dev;
>   adap->dev.of_node = hdmi->dev->of_node;
> diff --git a/drivers/gpu/drm/rockchip/rk3066_hdmi.c 
> b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> index fa6e592e0..7a3f71aa2 100644
> --- a/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/rk3066_hdmi.c
> @@ -725,7 +725,6 @@ static struct i2c_adapter *rk3066_hdmi_i2c_adapter(struct 
> rk3066_hdmi *hdmi)
>   init_completion(&i2c->cmpltn);
>  
>   adap = &i2c->adap;
> - adap->class = I2C_CLASS_DDC;
>   adap->owner = THIS_MODULE;
>   adap->dev.parent = hdmi->dev;
>   adap->dev.of_node = hdmi->dev->of_node;
> 
> 






Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

2023-10-24 Thread Heiko Stübner
Hi Chris,

Am Freitag, 20. Oktober 2023, 17:03:08 CEST schrieb Chris Morgan:
> On Thu, Oct 19, 2023 at 07:45:17PM +0200, Heiko Stübner wrote:
> > Hey Chris,
> > 
> > Am Donnerstag, 19. Oktober 2023, 16:43:56 CEST schrieb Chris Morgan:
> > > On Thu, Oct 19, 2023 at 11:21:47AM +0200, Krzysztof Kozlowski wrote:
> > > > On 18/10/2023 18:18, Chris Morgan wrote:
> > > > > From: Chris Morgan 
> > > > > 
> > > > > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > > > > powered by the Rockchip RK3566 SoC.
> > > > > 
> > > > > Signed-off-by: Chris Morgan 
> > > > > ---
> > > > >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +
> > > > >  1 file changed, 5 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml 
> > > > > b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > index a349bf4da6bc..a6612185a7ff 100644
> > > > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > > > @@ -674,6 +674,11 @@ properties:
> > > > >- const: powkiddy,rgb30
> > > > >- const: rockchip,rk3566
> > > > >  
> > > > > +  - description: Powkiddy RK2023
> > > > > +items:
> > > > > +  - const: powkiddy,rk2023
> > > > 
> > > > This cuold be just enum in previous entry :/ but I remember we talked
> > > > about this once with Heiko.
> > > 
> > > For hardware that requires a different device tree, is that possible?
> > > While most of the devices I've worked on for the RK3566 series are very
> > > similar for the moment only 1 is identical (the RG353P and the RG353M)
> > > and can use the same device tree.
> > 
> > In my reply I pointed to the Rock PI 4A/4A+/B/B+/C family, which also has
> > different devicetrees but is part of the same family of device designs.
> > 
> > So similar Powkiddy RK3568 based gaming handhelds also sound like
> > a nice family name in the description ;-) .
> 
> Gotcha, I can do that. Would you like for me to go back and do the same
> for the Anbernic devices as well? I can do it as part of a seperate
> patch series.

that doing that for the Anberic devices would be really nice too, so
yes please :-) .

Thanks
Heiko




Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

2023-10-19 Thread Heiko Stübner
Hey Chris,

Am Donnerstag, 19. Oktober 2023, 16:43:56 CEST schrieb Chris Morgan:
> On Thu, Oct 19, 2023 at 11:21:47AM +0200, Krzysztof Kozlowski wrote:
> > On 18/10/2023 18:18, Chris Morgan wrote:
> > > From: Chris Morgan 
> > > 
> > > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > > powered by the Rockchip RK3566 SoC.
> > > 
> > > Signed-off-by: Chris Morgan 
> > > ---
> > >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml 
> > > b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > index a349bf4da6bc..a6612185a7ff 100644
> > > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > > @@ -674,6 +674,11 @@ properties:
> > >- const: powkiddy,rgb30
> > >- const: rockchip,rk3566
> > >  
> > > +  - description: Powkiddy RK2023
> > > +items:
> > > +  - const: powkiddy,rk2023
> > 
> > This cuold be just enum in previous entry :/ but I remember we talked
> > about this once with Heiko.
> 
> For hardware that requires a different device tree, is that possible?
> While most of the devices I've worked on for the RK3566 series are very
> similar for the moment only 1 is identical (the RG353P and the RG353M)
> and can use the same device tree.

In my reply I pointed to the Rock PI 4A/4A+/B/B+/C family, which also has
different devicetrees but is part of the same family of device designs.

So similar Powkiddy RK3568 based gaming handhelds also sound like
a nice family name in the description ;-) .


Heiko




Re: [PATCH 4/5] dt-bindings: arm: rockchip: Add Powkiddy RK2023

2023-10-19 Thread Heiko Stübner
Am Donnerstag, 19. Oktober 2023, 11:21:47 CEST schrieb Krzysztof Kozlowski:
> On 18/10/2023 18:18, Chris Morgan wrote:
> > From: Chris Morgan 
> > 
> > The Powkiddy RK2023 is a handheld gaming device made by Powkiddy and
> > powered by the Rockchip RK3566 SoC.
> > 
> > Signed-off-by: Chris Morgan 
> > ---
> >  Documentation/devicetree/bindings/arm/rockchip.yaml | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml 
> > b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > index a349bf4da6bc..a6612185a7ff 100644
> > --- a/Documentation/devicetree/bindings/arm/rockchip.yaml
> > +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> > @@ -674,6 +674,11 @@ properties:
> >- const: powkiddy,rgb30
> >- const: rockchip,rk3566
> >  
> > +  - description: Powkiddy RK2023
> > +items:
> > +  - const: powkiddy,rk2023
> 
> This cuold be just enum in previous entry :/ but I remember we talked
> about this once with Heiko.

Keeping similar devices together is perfectly fine. Like we do for example
with the Rock PI 4A/4A+/B/B+/C family directly below.

The powkiddy,rk2023 really looks like very similar to the rgb30, so
could do something similar.


The variant I don't like is having one big enum for _all_  boards using
the same soc.


Heiko

> Acked-by: Krzysztof Kozlowski 
> 
> > +  - const: rockchip,rk3566
> 
> 
> 
> Best regards,
> Krzysztof
> 
> 






Re: [PATCH 5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc

2023-10-17 Thread Heiko Stübner
Hi,

Am Dienstag, 17. Oktober 2023, 12:15:11 CEST schrieb Ying Liu:
> On Tuesday, October 17, 2023 2:15 AM, Heiko Stübner  wrote:
> > Am Montag, 17. Juli 2023, 08:18:27 CEST schrieb Liu Ying:
> > > To get better accuration, use pixel clock rate to calculate lbcc instead 
> > > of
> > > lane_mbps since the pixel clock rate is in KHz while lane_mbps is in MHz.
> > > Without this, distorted image can be seen on a HDMI monitor connected
> > with
> > > i.MX93 11x11 EVK through ADV7535 DSI to HDMI bridge in 1920x1080p@60
> > video
> > > mode.
> > >
> > > Signed-off-by: Liu Ying 
> >
> > looks like I'm late to the party, but this change breaks the display output
> > my px30 minievb with the xinpeng xpp055c272 dsi display [0].
> 
> Hmm, I asked for a test, but anyway sorry for the breakage.

I'm often way behind with looking at drm-related changes, sorry about that.

So thanks a lot for taking the time to look into the problem.


> The panel driver sets MIPI_DSI_MODE_VIDEO_BURST.
> And, it seems that rockchip dsi driver [1] only supports the burst mode,
> because it takes 1/0.8 = 1.25 faster lane_mbps than "bandwidth of RGB".
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n568
> 
> >
> > Found this commit via git bisection and added a bit of debug output to
> > compare the value differences for the old and new calculation:
> >
> > [   34.810722] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 810 * 480 * 1000
> > / 8
> > [   34.810749] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 810 * 64000 *
> > 24 / (4 * 8)
> > [   34.810756] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 4860, new
> > lbcc: 3888
> > [   34.810762] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 10 * 480 * 1000 /
> > 8
> > [   34.810767] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 10 * 64000 * 24
> > / (4 * 8)
> > [   34.810773] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 60, new lbcc:
> > 48
> > [   34.810778] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 40 * 480 * 1000 /
> > 8
> > [   34.810783] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 40 * 64000 * 24
> > / (4 * 8)
> > [   34.810789] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 240, new
> > lbcc: 192
> 
> Old lbcc / new lbcc is always 1.25.
> 
> The new lbcc is for non-burst modes(sync pulse/sync event), IIUC.
> At least, it works for i.MX93 with the RM67191 panel and ADV7535 in
> sync pulse mode.
> 
> >
> > With the new lbcc I get a blank dsi panel and just going back to the old
> > calculation of lbcc restores the image.
> >
> > I don't have that much in-depth knowledge about dsi stuff and the original
> > panel times also "just" came from the vendor tree, but I really would like
> > to keep that display working ;-) .
> >
> > Do you have any idea which way to go to fix this?
> 
> Can you please test the below patch for your case?

The patch below does fix the display on the device. After applying it
I do get a working display again.


> 8<-
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -774,13 +774,19 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct 
> dw_mipi_dsi *dsi,
> u32 frac, lbcc, minimum_lbcc;
> int bpp;
> 
> -   bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> -   if (bpp < 0) {
> -   dev_err(dsi->dev, "failed to get bpp\n");
> -   return 0;
> -   }
> +   if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
> +   /* lbcc based on lane_mbps */
> +   lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
> +   } else {
> +   /* lbcc based on pixel clock */
> +   bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> +   if (bpp < 0) {
> +   dev_err(dsi->dev, "failed to get bpp\n");
> +   return 0;
> +   }
> 
> -   lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes * 8);
> +   lbcc = div_u64((u64)hcomponent * mode->clock * bpp, 
> dsi->lanes * 8);
> +   }
> 
> frac = lbcc % mode->clock;
> lbcc = lbcc / mode->clock;
> 8<-
> 

Re: [PATCH 3/5] drm/panel: st7703: Add Powkiddy RGB30 Panel Support

2023-10-17 Thread Heiko Stübner
Hi Chris,

Am Montag, 16. Oktober 2023, 20:26:58 CEST schrieb Chris Morgan:
> On Mon, Oct 16, 2023 at 08:18:25PM +0200, Heiko Stübner wrote:
> > Hi,
> > 
> > Am Montag, 16. Oktober 2023, 18:07:52 CEST schrieb Dragan Simic:
> > > On 2023-10-16 17:52, Chris Morgan wrote:
> > > > Confirmed that those pending patches DO fix the panel suspend issues. 
> > > > Thank you.
> > > 
> > > Awesome, that's great to hear!  Perhaps a "Tested-by" in the original 
> > > LKML thread [1] could help with having the patch pulled sooner.
> > > 
> > > Links:
> > > [1] 
> > > https://lore.kernel.org/lkml/33b72957-1062-1b66-85eb-c37dc5ca2...@redhat.com/T/
> > > 
> > > 
> > > > On Mon, Oct 16, 2023 at 3:41 AM Guido Günther  
> > > > wrote:
> > > >> 
> > > >> Hi Chris,
> > > >> On Fri, Oct 13, 2023 at 01:39:16PM -0500, Chris Morgan wrote:
> > > >> > From: Chris Morgan 
> > > >> >
> > > >> > The Powkiddy RGB30 4 inch panel is a 4 inch 720x720 DSI panel used in
> > > >> > the Powkiddy RGB30 handheld gaming device. Add support for it.
> > > >> >
> > > >> > TODO: The panel seems to not resume properly from suspend. I've
> > > >> > confirmed on the other ST7703 based devices it works correctly.
> > 
> > so this TODO item could go away, right?
> > I can remove it when applying the patch, just want to make sure
> > all review comments are addressed - only the suspend thing it seems.
> 
> That is correct, but let me send a v2 of this instead. I'll remove this
> verbiage among other fixes. End users wanted me to see if I could get
> this panel to run at precisely 60hz, which I believe I am able to do
> with the addition of a new PLL clock in clk_rk3568. I believe I have
> taken every constraint detailed in the datasheet to heart for the new
> frequency I'll be requesting. By using the frequency of 29250 for
> the VPLL I can get the panel running at 59.969hz which in my view is
> close enough to the ideal 59.98hz.
> 
> I also accidentally left the UART2 active even though this device has
> no exposed UART port, so I need to fix that too by disabling it.
> 
> Lastly I'll add my tested by to the dri-devel patches as well.

too late ;-)

Looks like your mail and me applying the series happened at a similar
time and I just saw your mail.

So if you want to change the dts now, please do a followup patch.

Thanks
Heiko




Re: [PATCH 3/5] drm/panel: st7703: Add Powkiddy RGB30 Panel Support

2023-10-16 Thread Heiko Stübner
Hi,

Am Montag, 16. Oktober 2023, 18:07:52 CEST schrieb Dragan Simic:
> On 2023-10-16 17:52, Chris Morgan wrote:
> > Confirmed that those pending patches DO fix the panel suspend issues. 
> > Thank you.
> 
> Awesome, that's great to hear!  Perhaps a "Tested-by" in the original 
> LKML thread [1] could help with having the patch pulled sooner.
> 
> Links:
> [1] 
> https://lore.kernel.org/lkml/33b72957-1062-1b66-85eb-c37dc5ca2...@redhat.com/T/
> 
> 
> > On Mon, Oct 16, 2023 at 3:41 AM Guido Günther  
> > wrote:
> >> 
> >> Hi Chris,
> >> On Fri, Oct 13, 2023 at 01:39:16PM -0500, Chris Morgan wrote:
> >> > From: Chris Morgan 
> >> >
> >> > The Powkiddy RGB30 4 inch panel is a 4 inch 720x720 DSI panel used in
> >> > the Powkiddy RGB30 handheld gaming device. Add support for it.
> >> >
> >> > TODO: The panel seems to not resume properly from suspend. I've
> >> > confirmed on the other ST7703 based devices it works correctly.

so this TODO item could go away, right?
I can remove it when applying the patch, just want to make sure
all review comments are addressed - only the suspend thing it seems.


Thanks
Heiko




Re: [PATCH 5/9] drm/bridge: synopsys: dw-mipi-dsi: Use pixel clock rate to calculate lbcc

2023-10-16 Thread Heiko Stübner
Hi,

Am Montag, 17. Juli 2023, 08:18:27 CEST schrieb Liu Ying:
> To get better accuration, use pixel clock rate to calculate lbcc instead of
> lane_mbps since the pixel clock rate is in KHz while lane_mbps is in MHz.
> Without this, distorted image can be seen on a HDMI monitor connected with
> i.MX93 11x11 EVK through ADV7535 DSI to HDMI bridge in 1920x1080p@60 video
> mode.
> 
> Signed-off-by: Liu Ying 

looks like I'm late to the party, but this change breaks the display output
my px30 minievb with the xinpeng xpp055c272 dsi display [0].

Found this commit via git bisection and added a bit of debug output to
compare the value differences for the old and new calculation:

[   34.810722] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 810 * 480 * 1000 / 8
[   34.810749] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 810 * 64000 * 24 / (4 
* 8)
[   34.810756] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 4860, new lbcc: 
3888
[   34.810762] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 10 * 480 * 1000 / 8
[   34.810767] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 10 * 64000 * 24 / (4 
* 8)
[   34.810773] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 60, new lbcc: 
48
[   34.810778] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 40 * 480 * 1000 / 8
[   34.810783] dw_mipi_dsi_get_hcomponent_lbcc: new lbcc: 40 * 64000 * 24 / (4 
* 8)
[   34.810789] dw_mipi_dsi_get_hcomponent_lbcc: old lbcc: 240, new lbcc: 
192

With the new lbcc I get a blank dsi panel and just going back to the old
calculation of lbcc restores the image.

I don't have that much in-depth knowledge about dsi stuff and the original
panel times also "just" came from the vendor tree, but I really would like
to keep that display working ;-) .

Do you have any idea which way to go to fix this?


Thanks
Heiko


[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/px30-evb.dts#n138
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index c754d55f71d1..332388fd86da 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -12,6 +12,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -762,8 +763,15 @@ static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct 
> dw_mipi_dsi *dsi,
>  u32 hcomponent)
>  {
>   u32 frac, lbcc;
> + int bpp;
>  
> - lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
> + bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> + if (bpp < 0) {
> + dev_err(dsi->dev, "failed to get bpp\n");
> + return 0;
> + }
> +
> + lbcc = div_u64((u64)hcomponent * mode->clock * bpp, dsi->lanes * 8);






Re: [PATCH 1/3] drm/panel: ltk050h3146w: add mipi_dsi_device.mode_flags to of_match_data

2023-10-10 Thread Heiko Stübner
Am Dienstag, 10. Oktober 2023, 18:54:11 CEST schrieb Heiko Stuebner:
> On Mon, 31 Jan 2022 17:47:21 +0100, quentin.sch...@theobroma-systems.com 
> wrote:
> > From: Quentin Schulz 
> > 
> > To prepare for a new display to be supported by this driver which has a
> > slightly different set of DSI mode related flags, let's move the
> > currently hardcoded mode flags to the .data field of of_device_id
> > structure.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/3] drm/panel: ltk050h3146w: add mipi_dsi_device.mode_flags to of_match_data
>   commit: 99403d747ae8c7b3bfb5cd14c8908930ec6801c6
> [2/3] drm/panel: ltk050h3146w: add support for Leadtek LTK050H3148W-CTA6 
> variant
>   commit: e5f9d543419c78ac58f3b3557bc5a76b20ff600b
> [3/3] dt-bindings: ltk050h3146w: add compatible for LTK050H3148W-CTA6 variant
>   commit: 29d8e38c36cb662331a833699c359ec1af1edbec

forgot to add, I included the move to the generic mipi_dsi_dcs_write_seq()
for the new sequence.






Re: [PATCH v2 0/5] drm/rockchip: Fix crtc duplicate state and crtc reset funcs

2023-08-12 Thread Heiko Stübner
Hi Jonas,

Am Samstag, 12. August 2023, 16:18:05 CEST schrieb Jonas Karlman:
> Please consider reviewing and merging this series [2], and also [3].

during the last months my testfarm aquired some issues, I'm still
working on fixing, so my testing is way limited right now.

> drm/rockchip: Fix crtc duplicate state and crtc reset funcs
> [2] https://lore.kernel.org/all/20230621223311.2239547-1-jo...@kwiboo.se/

Though this one looked easy and when going through the code looked
quite right.


> drm/rockchip: vop: Add NV15, NV20 and NV30 support
> [3] https://lore.kernel.org/all/20230618220122.3911297-1-jo...@kwiboo.se/

I guess I need to track down someone on IRC to tell me if these new NVxx
types look correct, because I don't have too much clue about those drm-formats
yet. Hopefully I'll get to that on monday.


Heiko


> On 2023-06-22 00:33, Jonas Karlman wrote:
> > This series fixes a reset of state in duplicate state crtc funcs for VOP
> > driver, a possible crash and ensure crtc reset helper is called in VOP2
> > driver.
> > 
> > Patch 1 use kmemdup instead of kzalloc to duplicate the crtc state.
> > Patch 2 change to use crtc and plane cleanup helpers directly.
> > Patch 3 adds a null guard for allocation failure.
> > Patch 4 adds a crash guard for empty crtc state.
> > Patch 5 adds a missing call to crtc reset helper.
> > 
> > This is the next part of an ongoing effort to upstream HDMI 2.0 support
> > used in LibreELEC for the past 3+ years.
> > 
> > Changes in v2:
> > - Handle possible allocation failure in crtc reset funcs
> > - Collect r-b tags
> > 
> > This series is also available at [1].
> > 
> > [1] 
> > https://github.com/Kwiboo/linux-rockchip/commits/next-20230621-duplicate-state
> > 
> > Jonas Karlman (5):
> >   drm/rockchip: vop: Fix reset of state in duplicate state crtc funcs
> >   drm/rockchip: vop: Use cleanup helper directly as destroy funcs
> >   drm/rockchip: vop: Fix call to crtc reset helper
> >   drm/rockchip: vop2: Don't crash for invalid duplicate_state
> >   drm/rockchip: vop2: Add missing call to crtc reset helper
> > 
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c  | 24 +---
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 39 ++--
> >  2 files changed, 28 insertions(+), 35 deletions(-)
> > 
> 
> 






Re: [PATCH 2/3] dt-bindings: backlight: lm3630a: add entries to control boost frequency

2023-06-17 Thread Heiko Stübner
Am Samstag, 17. Juni 2023, 12:12:17 CEST schrieb Krzysztof Kozlowski:
> On 14/06/2023 21:08, Maximilian Weigand wrote:
> > From: Maximilian Weigand 
> > 
> > Add 'ti,boost_use_1mhz' to switch between 500 kHz and 1 MHz boost
> > converter switching frequency, and add 'ti,boost_frequency_shift' to
> > activate a frequency shift to 560 kHz or 1.12 MHz, respectively.
> > 
> > Signed-off-by: Maximilian Weigand 
> > ---
> >  .../bindings/leds/backlight/lm3630a-backlight.yaml   | 12 
> > 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml 
> > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > index 3c9b4054ed9a..ef7ea0ad2d25 100644
> > --- 
> > a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > +++ 
> > b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> > @@ -33,6 +33,18 @@ properties:
> >  description: GPIO to use to enable/disable the backlight (HWEN pin).
> >  maxItems: 1
> >  
> > +  ti,boost_use_1mhz:
> 
> No underscores in property names.
> 
> > +description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
> > +  If present, change the boost converter switching frequency from the
> > +  default 500 kHz to 1 MHz. Refer to data sheet for hardware 
> > requirements.
> > +type: boolean
> > +
> > +  ti,boost_frequency_shift:
> > +description: |
> > +  If present, change boost converter switching frequency from 500 kHz 
> > to
> > +  560 kHz or from 1 Mhz to 1.12 Mhz, respectively.
> 
> So just make it a property choosing the frequency, not bools, with
> proper unit suffix.

i.e.
ti,boost-frequency-hz = ;
with x being 50, 56, 100, 112

with the driver failing when the frequency is not achievable
with the two knobs of 1mhz and shift.





Re: [PATCH 1/9] dt-bindings: display: Add yamls for JH7110 display subsystem

2023-06-07 Thread Heiko Stübner
Am Mittwoch, 7. Juni 2023, 00:37:53 CEST schrieb Conor Dooley:
> On Wed, Jun 07, 2023 at 12:22:33AM +0200, Heiko Stübner wrote:
> > Am Dienstag, 6. Juni 2023, 20:41:17 CEST schrieb Shengyu Qu:
> > > > On Fri, Jun 02, 2023 at 03:40:35PM +0800, Keith Zhao wrote:
> > > >> Add bindings for JH7110 display subsystem which
> > > >> has a display controller verisilicon dc8200
> > > >> and an HDMI interface.
> 
> > > >> +description:
> > > >> +  The StarFive SoC uses the HDMI signal transmiter based on 
> > > >> innosilicon IP
> > > > Is innosilicon the same thing as verisilicon? Also
> > > > s/transmiter/transmitter/, both here and in the title.
> > > 
> > > I think that is not the same, I remember Rockchip has used a HDMI 
> > > transmitter from
> > > 
> > > Innosilicon, and there is a existing driver for that in mainline.
> > 
> > Yep, I think Innosilicon is the company you turn to when you want to save
> > a bit of money ;-) . In the bigger SoCs Rockchip most of the time uses
> > Designware hdmi blocks and looking at the history only the rk3036 ever
> > used an Innosilicon block.
> > 
> > Looking at the history, 2016 really was a long time ago :-D.
> > 
> > > So Keith, if that's true, I think it is better to seperate the HDMI 
> > > stuff and reuse existing driver.
> > 
> > I'm not so sure about that - at least from a cursory glance :-) .
> > 
> > The registers do look slightly different and I don't know how much
> > the IP changed between the rk3036-version and the jh7110 version.
> > 
> > At the very least, I know my rk3036 board isn't booting right now, so
> > I can't really provide help for generalizing the rockchip-driver.
> > 
> > At the very least both the binding and driver could drop the "starfive-hdmi"
> > and actually use the Innosilicon in the naming somewhere, so that it's
> > clear for future developers :-)
> 
> Seeing "based on" always makes me a little bit nervous to be honest when
> it comes to using a compatible from the IP. Is it the IP? What version
> is it? etc. Perhaps "starfive,jh7110-hdmi" & falling back to some sort
> of "innosilicon,hdmi" would be more future/IP-silliness proof.
> Driver can always be generic & bind against "innosilicon,hdmi" until
> that becomes impossible.


what Connor said makes a lot of sense. Just name the compatible
after the actual implementation - aka "starfive,jh7110-hdmi" .

This is similar to what the rk3036 does with its
"rockchip,rk3036-inno-hdmi". That way you're nicely independent
and future proof.


Heiko




Re: [PATCH 1/9] dt-bindings: display: Add yamls for JH7110 display subsystem

2023-06-06 Thread Heiko Stübner
Am Dienstag, 6. Juni 2023, 20:41:17 CEST schrieb Shengyu Qu:
> Hi Conor,
> 
> > Hey Keith,
> >
> > On Fri, Jun 02, 2023 at 03:40:35PM +0800, Keith Zhao wrote:
> >> Add bindings for JH7110 display subsystem which
> >> has a display controller verisilicon dc8200
> >> and an HDMI interface.
> >>
> >> Signed-off-by: Keith Zhao 
> >> ---
> >>   .../display/verisilicon/starfive-hdmi.yaml|  93 +++
> >>   .../display/verisilicon/verisilicon-dc.yaml   | 110 ++
> >>   .../display/verisilicon/verisilicon-drm.yaml  |  42 +++
> >>   .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
> >>   MAINTAINERS   |   7 ++
> >>   5 files changed, 254 insertions(+)
> >>   create mode 100644 
> >> Documentation/devicetree/bindings/display/verisilicon/starfive-hdmi.yaml
> >>   create mode 100644 
> >> Documentation/devicetree/bindings/display/verisilicon/verisilicon-dc.yaml
> >>   create mode 100644 
> >> Documentation/devicetree/bindings/display/verisilicon/verisilicon-drm.yaml
> >>
> >> diff --git 
> >> a/Documentation/devicetree/bindings/display/verisilicon/starfive-hdmi.yaml 
> >> b/Documentation/devicetree/bindings/display/verisilicon/starfive-hdmi.yaml
> >> new file mode 100644
> >> index ..c30b7954a355
> >> --- /dev/null
> >> +++ 
> >> b/Documentation/devicetree/bindings/display/verisilicon/starfive-hdmi.yaml
> >> @@ -0,0 +1,93 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/display/verisilicon/starfive-hdmi.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: StarFive HDMI transmiter
> >> +
> >> +description:
> >> +  The StarFive SoC uses the HDMI signal transmiter based on innosilicon IP
> > Is innosilicon the same thing as verisilicon? Also
> > s/transmiter/transmitter/, both here and in the title.
> 
> I think that is not the same, I remember Rockchip has used a HDMI 
> transmitter from
> 
> Innosilicon, and there is a existing driver for that in mainline.

Yep, I think Innosilicon is the company you turn to when you want to save
a bit of money ;-) . In the bigger SoCs Rockchip most of the time uses
Designware hdmi blocks and looking at the history only the rk3036 ever
used an Innosilicon block.

Looking at the history, 2016 really was a long time ago :-D.


> So Keith, if that's true, I think it is better to seperate the HDMI 
> stuff and reuse existing driver.

I'm not so sure about that - at least from a cursory glance :-) .

The registers do look slightly different and I don't know how much
the IP changed between the rk3036-version and the jh7110 version.

At the very least, I know my rk3036 board isn't booting right now, so
I can't really provide help for generalizing the rockchip-driver.


At the very least both the binding and driver could drop the "starfive-hdmi"
and actually use the Innosilicon in the naming somewhere, so that it's
clear for future developers :-)


Heiko


> >> +  to generate HDMI signal from its input and transmit the signal to the 
> >> screen.
> >> +
> >> +maintainers:
> >> +  - Keith Zhao 
> >> +  - ShengYang Chen 
> >> +
> >> +properties:
> >> +  compatible:
> >> +const: starfive,hdmi
> > Is this going to work on every SoC that StarFive has ever & will ever
> > make? Please use soc-based compatibles ;)
> >
> >> +
> >> +  reg:
> >> +minItems: 1
> >> +
> >> +  interrupts:
> >> +items:
> >> +  - description: The HDMI hot plug detection interrupt.
> >> +
> >> +  clocks:
> >> +items:
> >> +  - description: System clock of HDMI module.
> >> +  - description: Mclk clock of HDMI audio.
> >> +  - description: Bclk clock of HDMI audio.
> >> +  - description: Pixel clock generated by HDMI module.
> >> +
> >> +  clock-names:
> >> +items:
> >> +  - const: sysclk
> >> +  - const: mclk
> >> +  - const: bclk
> >> +  - const: pclk
> >> +
> >> +  resets:
> >> +items:
> >> +  - description: Reset for HDMI module.
> >> +
> >> +  reset-names:
> >> +items:
> >> +  - const: hdmi_tx
> > You only have one item here, you don't need the "items: - const:",
> > "const:" alone will do.
> >
> >
> >> diff --git 
> >> a/Documentation/devicetree/bindings/display/verisilicon/verisilicon-dc.yaml
> >>  
> >> b/Documentation/devicetree/bindings/display/verisilicon/verisilicon-dc.yaml
> >> new file mode 100644
> >> index ..1322502c4cde
> >> --- /dev/null
> >> +++ 
> >> b/Documentation/devicetree/bindings/display/verisilicon/verisilicon-dc.yaml
> >> @@ -0,0 +1,110 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: 
> >> http://devicetree.org/schemas/display/verisilicon/verisilicon-dc.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: StarFive display controller
> >> +
> >> +description:
> >> +  The StarFive SoC uses the display controller based on Verisilicon IP

Re: [PATCH 38/53] drm/rockchip: Convert to platform remove callback returning void

2023-05-08 Thread Heiko Stübner
Am Sonntag, 7. Mai 2023, 18:26:01 CEST schrieb Uwe Kleine-König:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert rockchip drm drivers from always returning zero in the
> remove callback to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König 

Acked-by: Heiko Stuebner 




Re: [PATCH] drm/rockchip: vop2: fix suspend/resume

2023-04-17 Thread Heiko Stübner
Hi Sascha,

Am Montag, 17. April 2023, 11:42:15 CEST schrieb Sascha Hauer:
> During a suspend/resume cycle the VO power domain will be disabled and
> the VOP2 registers will reset to their default values. After that the
> cached register values will be out of sync and the read/modify/write
> operations we do on the window registers will result in bogus values
> written. Fix this by marking the regcache as dirty each time we disable
> the VOP2 and call regcache_sync() each time we enable it again. With
> this the VOP2 will show a picture after a suspend/resume cycle whereas
> without this the screen stays dark.
> 
> Fixes: 604be85547ce4 ("drm/rockchip: Add VOP2 driver")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Sascha Hauer 

somehow we overlapped with this v2 and me applying the original one [0]
to drm-misc. With drm-misc being a shared tree there is also no way back.

So if this v2 is better suited, could do a follow-up patch instead - on
top of your original one?

Thanks
Heiko



[0] 
https://cgit.freedesktop.org/drm/drm-misc/commit/?h=drm-misc-fixes&id=afa965a45e01e541cdbe5c8018226eff117610f0

> ---
> 
> Changes since v1:
> - Use regcache_mark_dirty()/regcache_sync() instead of regmap_reinit_cache()
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index ba3b817895091..293c228a83f90 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -839,6 +839,8 @@ static void vop2_enable(struct vop2 *vop2)
>   return;
>   }
>  
> + regcache_sync(vop2->map);
> +
>   if (vop2->data->soc_id == 3566)
>   vop2_writel(vop2, RK3568_OTP_WIN_EN, 1);
>  
> @@ -867,6 +869,8 @@ static void vop2_disable(struct vop2 *vop2)
>  
>   pm_runtime_put_sync(vop2->dev);
>  
> + regcache_mark_dirty(vop2->map);
> +
>   clk_disable_unprepare(vop2->aclk);
>   clk_disable_unprepare(vop2->hclk);
>  }
> 






Re: [PATCH 11/12] drm/rockchip: convert to using has_audio from display_info

2023-03-30 Thread Heiko Stübner
Am Donnerstag, 30. März 2023, 17:39:48 CEST schrieb Jani Nikula:
> Prefer the parsed results for has_audio in display info over calling
> drm_detect_monitor_audio().
> 
> Cc: Sandy Huang 
> Cc: Heiko Stübner 
> Signed-off-by: Jani Nikula 

Acked-by: Heiko Stuebner 

> ---
>  drivers/gpu/drm/rockchip/cdn-dp-core.c | 4 ++--
>  drivers/gpu/drm/rockchip/inno_hdmi.c   | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c 
> b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index b6afe3786b74..4a4cf4354e27 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -272,10 +272,10 @@ static int cdn_dp_connector_get_modes(struct 
> drm_connector *connector)
>   DRM_DEV_DEBUG_KMS(dp->dev, "got edid: width[%d] x height[%d]\n",
> edid->width_cm, edid->height_cm);
>  
> - dp->sink_has_audio = drm_detect_monitor_audio(edid);
> -
>   drm_connector_update_edid_property(connector, edid);
>   ret = drm_add_edid_modes(connector, edid);
> +
> + dp->sink_has_audio = connector->display_info.has_audio;
>   }
>   mutex_unlock(&dp->lock);
>  
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c 
> b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index f51774866f41..98691aef1be5 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -564,10 +564,11 @@ static int inno_hdmi_connector_get_modes(struct 
> drm_connector *connector)
>  
>   edid = drm_get_edid(connector, hdmi->ddc);
>   if (edid) {
> - hdmi->hdmi_data.sink_has_audio = drm_detect_monitor_audio(edid);
>   drm_connector_update_edid_property(connector, edid);
>   ret = drm_add_edid_modes(connector, edid);
>   kfree(edid);
> +
> + hdmi->hdmi_data.sink_has_audio = 
> connector->display_info.has_audio;
>   }
>  
>   return ret;
> 






Re: [PATCH 10/12] drm/rockchip: cdn-dp: call drm_connector_update_edid_property() unconditionally

2023-03-30 Thread Heiko Stübner
Am Donnerstag, 30. März 2023, 17:39:47 CEST schrieb Jani Nikula:
> Calling drm_connector_update_edid_property() should be done
> unconditionally instead of depending on the number of modes added. Also
> match the call order in inno_hdmi and rk3066_hdmi.
> 
> Cc: Sandy Huang 
> Cc: Heiko Stübner 
> Signed-off-by: Jani Nikula 

Acked-by: Heiko Stuebner 

> ---
>  drivers/gpu/drm/rockchip/cdn-dp-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c 
> b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 8526dda91931..b6afe3786b74 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -273,10 +273,9 @@ static int cdn_dp_connector_get_modes(struct 
> drm_connector *connector)
> edid->width_cm, edid->height_cm);
>  
>   dp->sink_has_audio = drm_detect_monitor_audio(edid);
> +
> + drm_connector_update_edid_property(connector, edid);
>   ret = drm_add_edid_modes(connector, edid);
> - if (ret)
> - drm_connector_update_edid_property(connector,
> - edid);
>   }
>   mutex_unlock(&dp->lock);
>  
> 






  1   2   3   4   >