| Message ID | 20200330181613.29462-2-jagan@amarulasolutions.com |
|---|---|
| State | Superseded |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
| Series | rockchip: rk3399: Fix HDMI out | expand |
> From: Jagan Teki <jagan@amarulasolutions.com> > Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de, > linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com, > Jagan Teki <jagan@amarulasolutions.com> > Date: Mon, 30 Mar 2020 23:46:10 +0530 > Content-Type: text/plain; charset=UTF-8 > > Linux supporting assigned-clocks for VOP on rk3399 by assuming > U-Boot not initializing it on this linux commit: > > commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates") > > There is no specific need to initialize these assigned clock > in U-Boot as video drivers still work with default aclk and > hclk values. So, these clocks are simply not supported by rk3399 > clock driver. > > But, during stdio probe of vidconsole, the device probe > will try to check whether the assigned clocks on that video > console node is initialized or not? and return error if not. > > So, delete these property via -u-boot dtsi as there is > no specific need in U-Boot. Deleting these properties isn't very helpful as it means the U-Boot device tree can no longer be used by the kernel. Isn't it a better idea to implement these clocks as stubs in the u-boot clock driver? > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > Changes for v2: > - none > > arch/arm/dts/rk3399-u-boot.dtsi | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi > index 8b857ccfc7..b846f9cde7 100644 > --- a/arch/arm/dts/rk3399-u-boot.dtsi > +++ b/arch/arm/dts/rk3399-u-boot.dtsi > @@ -99,9 +99,13 @@ > }; > > &vopb { > + /delete-property/ assigned-clocks; > + /delete-property/ assigned-clock-rates; > u-boot,dm-pre-reloc; > }; > > &vopl { > + /delete-property/ assigned-clocks; > + /delete-property/ assigned-clock-rates; > u-boot,dm-pre-reloc; > }; > -- > 2.17.1 > >
On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Jagan Teki <jagan@amarulasolutions.com> > > Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de, > > linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com, > > Jagan Teki <jagan@amarulasolutions.com> > > Date: Mon, 30 Mar 2020 23:46:10 +0530 > > Content-Type: text/plain; charset=UTF-8 > > > > Linux supporting assigned-clocks for VOP on rk3399 by assuming > > U-Boot not initializing it on this linux commit: > > > > commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates") > > > > There is no specific need to initialize these assigned clock > > in U-Boot as video drivers still work with default aclk and > > hclk values. So, these clocks are simply not supported by rk3399 > > clock driver. > > > > But, during stdio probe of vidconsole, the device probe > > will try to check whether the assigned clocks on that video > > console node is initialized or not? and return error if not. > > > > So, delete these property via -u-boot dtsi as there is > > no specific need in U-Boot. > > Deleting these properties isn't very helpful as it means the U-Boot > device tree can no longer be used by the kernel. Isn't it a better > idea to implement these clocks as stubs in the u-boot clock driver? I did try this before sorting out these changes, seems like it requires a bit more tweaking the clock wrt display code. I really didn't see any use case as of now for just to print u-boot log on display out, and more over this support has been broken since from releases. so bypassing these nodes can be a solutions for now. Jagan.
Hi Jagan, On 2020/3/31 下午1:59, Jagan Teki wrote: > On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: >>> From: Jagan Teki <jagan@amarulasolutions.com> >>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de, >>> linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com, >>> Jagan Teki <jagan@amarulasolutions.com> >>> Date: Mon, 30 Mar 2020 23:46:10 +0530 >>> Content-Type: text/plain; charset=UTF-8 >>> >>> Linux supporting assigned-clocks for VOP on rk3399 by assuming >>> U-Boot not initializing it on this linux commit: >>> >>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates") >>> >>> There is no specific need to initialize these assigned clock >>> in U-Boot as video drivers still work with default aclk and >>> hclk values. So, these clocks are simply not supported by rk3399 >>> clock driver. >>> >>> But, during stdio probe of vidconsole, the device probe >>> will try to check whether the assigned clocks on that video >>> console node is initialized or not? and return error if not. >>> >>> So, delete these property via -u-boot dtsi as there is >>> no specific need in U-Boot. >> Deleting these properties isn't very helpful as it means the U-Boot >> device tree can no longer be used by the kernel. Isn't it a better >> idea to implement these clocks as stubs in the u-boot clock driver? > I did try this before sorting out these changes, seems like it > requires a bit more tweaking the clock wrt display code. I really > didn't see any use case as of now for just to print u-boot log on > display out, and more over this support has been broken since from > releases. so bypassing these nodes can be a solutions for now. I agree with Mark for not touch the dts first. I don't know the detail of display driver but: - The rk3399 driver use to work without touch dts from kernel; - the clock driver have a rk3399_vop_set_clk() which does not depends on dts. Thanks, - Kever > > Jagan. > >
Hi Kever, On Thu, Apr 2, 2020 at 2:48 PM Kever Yang <kever.yang@rock-chips.com> wrote: > > Hi Jagan, > > On 2020/3/31 下午1:59, Jagan Teki wrote: > > On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > >>> From: Jagan Teki <jagan@amarulasolutions.com> > >>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de, > >>> linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com, > >>> Jagan Teki <jagan@amarulasolutions.com> > >>> Date: Mon, 30 Mar 2020 23:46:10 +0530 > >>> Content-Type: text/plain; charset=UTF-8 > >>> > >>> Linux supporting assigned-clocks for VOP on rk3399 by assuming > >>> U-Boot not initializing it on this linux commit: > >>> > >>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates") > >>> > >>> There is no specific need to initialize these assigned clock > >>> in U-Boot as video drivers still work with default aclk and > >>> hclk values. So, these clocks are simply not supported by rk3399 > >>> clock driver. > >>> > >>> But, during stdio probe of vidconsole, the device probe > >>> will try to check whether the assigned clocks on that video > >>> console node is initialized or not? and return error if not. > >>> > >>> So, delete these property via -u-boot dtsi as there is > >>> no specific need in U-Boot. > >> Deleting these properties isn't very helpful as it means the U-Boot > >> device tree can no longer be used by the kernel. Isn't it a better > >> idea to implement these clocks as stubs in the u-boot clock driver? > > I did try this before sorting out these changes, seems like it > > requires a bit more tweaking the clock wrt display code. I really > > didn't see any use case as of now for just to print u-boot log on > > display out, and more over this support has been broken since from > > releases. so bypassing these nodes can be a solutions for now. > > > I agree with Mark for not touch the dts first. I don't know the detail > of display driver but: > > - The rk3399 driver use to work without touch dts from kernel; > > - the clock driver have a rk3399_vop_set_clk() which does not depends on > dts. The existing video drivers are written based on the puma dts and those are not inline to Linux dts files, i.e. the reason the code is pushed I think. The rest of rk3399 dtsi files are now inline to Linux as and display out on these are broken from last 2 releases. so my idea is to resolve the things one-after-another like 1. Make existing video stuff work with all rk3399 (this series along with this patch) 2. Drop this patch change and make video drivers working w/o any explicit changes in dts like this patch does. Since step 2, would take time, and require close testing of all boards I would like to pick the existing stuff for the release. Mark my words to fix the things for the next release. Jagan.
> From: Jagan Teki <jagan@amarulasolutions.com> > Date: Thu, 2 Apr 2020 15:07:01 +0530 > > Hi Kever, > > On Thu, Apr 2, 2020 at 2:48 PM Kever Yang <kever.yang@rock-chips.com> wrote: > > > > Hi Jagan, > > > > On 2020/3/31 下午1:59, Jagan Teki wrote: > > > On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > >>> From: Jagan Teki <jagan@amarulasolutions.com> > > >>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de, > > >>> linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com, > > >>> Jagan Teki <jagan@amarulasolutions.com> > > >>> Date: Mon, 30 Mar 2020 23:46:10 +0530 > > >>> Content-Type: text/plain; charset=UTF-8 > > >>> > > >>> Linux supporting assigned-clocks for VOP on rk3399 by assuming > > >>> U-Boot not initializing it on this linux commit: > > >>> > > >>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates") > > >>> > > >>> There is no specific need to initialize these assigned clock > > >>> in U-Boot as video drivers still work with default aclk and > > >>> hclk values. So, these clocks are simply not supported by rk3399 > > >>> clock driver. > > >>> > > >>> But, during stdio probe of vidconsole, the device probe > > >>> will try to check whether the assigned clocks on that video > > >>> console node is initialized or not? and return error if not. > > >>> > > >>> So, delete these property via -u-boot dtsi as there is > > >>> no specific need in U-Boot. > > >> Deleting these properties isn't very helpful as it means the U-Boot > > >> device tree can no longer be used by the kernel. Isn't it a better > > >> idea to implement these clocks as stubs in the u-boot clock driver? > > > I did try this before sorting out these changes, seems like it > > > requires a bit more tweaking the clock wrt display code. I really > > > didn't see any use case as of now for just to print u-boot log on > > > display out, and more over this support has been broken since from > > > releases. so bypassing these nodes can be a solutions for now. > > > > > > I agree with Mark for not touch the dts first. I don't know the detail > > of display driver but: > > > > - The rk3399 driver use to work without touch dts from kernel; > > > > - the clock driver have a rk3399_vop_set_clk() which does not depends on > > dts. > > The existing video drivers are written based on the puma dts and those > are not inline to Linux dts files, i.e. the reason the code is pushed > I think. The rest of rk3399 dtsi files are now inline to Linux as and > display out on these are broken from last 2 releases. so my idea is to > resolve the things one-after-another like > 1. Make existing video stuff work with all rk3399 (this series along > with this patch) > 2. Drop this patch change and make video drivers working w/o any > explicit changes in dts like this patch does. > > Since step 2, would take time, and require close testing of all boards > I would like to pick the existing stuff for the release. Mark my words > to fix the things for the next release. Fair enough. I don't think fixing the issue is too difficult, but it is better to do these things in small steps anyway.
On Thu, Apr 2, 2020 at 3:48 PM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > From: Jagan Teki <jagan@amarulasolutions.com> > > Date: Thu, 2 Apr 2020 15:07:01 +0530 > > > > Hi Kever, > > > > On Thu, Apr 2, 2020 at 2:48 PM Kever Yang <kever.yang@rock-chips.com> wrote: > > > > > > Hi Jagan, > > > > > > On 2020/3/31 下午1:59, Jagan Teki wrote: > > > > On Tue, Mar 31, 2020 at 1:06 AM Mark Kettenis <mark.kettenis@xs4all.nl> wrote: > > > >>> From: Jagan Teki <jagan@amarulasolutions.com> > > > >>> Cc: sunil@amarulasolutions.com, u-boot@lists.denx.de, > > > >>> linux-rockchip@lists.infradead.org, linux-amarula@amarulasolutions.com, > > > >>> Jagan Teki <jagan@amarulasolutions.com> > > > >>> Date: Mon, 30 Mar 2020 23:46:10 +0530 > > > >>> Content-Type: text/plain; charset=UTF-8 > > > >>> > > > >>> Linux supporting assigned-clocks for VOP on rk3399 by assuming > > > >>> U-Boot not initializing it on this linux commit: > > > >>> > > > >>> commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates") > > > >>> > > > >>> There is no specific need to initialize these assigned clock > > > >>> in U-Boot as video drivers still work with default aclk and > > > >>> hclk values. So, these clocks are simply not supported by rk3399 > > > >>> clock driver. > > > >>> > > > >>> But, during stdio probe of vidconsole, the device probe > > > >>> will try to check whether the assigned clocks on that video > > > >>> console node is initialized or not? and return error if not. > > > >>> > > > >>> So, delete these property via -u-boot dtsi as there is > > > >>> no specific need in U-Boot. > > > >> Deleting these properties isn't very helpful as it means the U-Boot > > > >> device tree can no longer be used by the kernel. Isn't it a better > > > >> idea to implement these clocks as stubs in the u-boot clock driver? > > > > I did try this before sorting out these changes, seems like it > > > > requires a bit more tweaking the clock wrt display code. I really > > > > didn't see any use case as of now for just to print u-boot log on > > > > display out, and more over this support has been broken since from > > > > releases. so bypassing these nodes can be a solutions for now. > > > > > > > > > I agree with Mark for not touch the dts first. I don't know the detail > > > of display driver but: > > > > > > - The rk3399 driver use to work without touch dts from kernel; > > > > > > - the clock driver have a rk3399_vop_set_clk() which does not depends on > > > dts. > > > > The existing video drivers are written based on the puma dts and those > > are not inline to Linux dts files, i.e. the reason the code is pushed > > I think. The rest of rk3399 dtsi files are now inline to Linux as and > > display out on these are broken from last 2 releases. so my idea is to > > resolve the things one-after-another like > > 1. Make existing video stuff work with all rk3399 (this series along > > with this patch) > > 2. Drop this patch change and make video drivers working w/o any > > explicit changes in dts like this patch does. > > > > Since step 2, would take time, and require close testing of all boards > > I would like to pick the existing stuff for the release. Mark my words > > to fix the things for the next release. > > Fair enough. I don't think fixing the issue is too difficult, but it > is better to do these things in small steps anyway. I have managed to fix this via clock driver(which seems more reasonable as per Kever), so this is dropped in v3. thanks for noticing. Jagan.
diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi index 8b857ccfc7..b846f9cde7 100644 --- a/arch/arm/dts/rk3399-u-boot.dtsi +++ b/arch/arm/dts/rk3399-u-boot.dtsi @@ -99,9 +99,13 @@ }; &vopb { + /delete-property/ assigned-clocks; + /delete-property/ assigned-clock-rates; u-boot,dm-pre-reloc; }; &vopl { + /delete-property/ assigned-clocks; + /delete-property/ assigned-clock-rates; u-boot,dm-pre-reloc; };
Linux supporting assigned-clocks for VOP on rk3399 by assuming U-Boot not initializing it on this linux commit: commit <617f4472bdd3> ("arm64: dts: rockchip: init rk3399 vop clock rates") There is no specific need to initialize these assigned clock in U-Boot as video drivers still work with default aclk and hclk values. So, these clocks are simply not supported by rk3399 clock driver. But, during stdio probe of vidconsole, the device probe will try to check whether the assigned clocks on that video console node is initialized or not? and return error if not. So, delete these property via -u-boot dtsi as there is no specific need in U-Boot. Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- Changes for v2: - none arch/arm/dts/rk3399-u-boot.dtsi | 4 ++++ 1 file changed, 4 insertions(+)