diff mbox series

drm/omap: Migrate minimum FCK/PCK ratio from Kconfig to dts

Message ID 20190510194229.20628-1-aford173@gmail.com
State Changes Requested, archived
Headers show
Series drm/omap: Migrate minimum FCK/PCK ratio from Kconfig to dts | expand

Checks

Context Check Description
robh/checkpatch warning "total: 0 errors, 1 warnings, 80 lines checked"

Commit Message

Adam Ford May 10, 2019, 7:42 p.m. UTC
Currently the source code is compiled using hard-coded values
from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
clock divider value to be moved to the device tree and be changed
without having to recompile the kernel.

Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Tomi Valkeinen May 28, 2019, 11:11 a.m. UTC | #1
Hi,

On 10/05/2019 22:42, Adam Ford wrote:
> Currently the source code is compiled using hard-coded values
> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
> clock divider value to be moved to the device tree and be changed
> without having to recompile the kernel.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>

I understand why you want to do this, but I'm not sure it's a good idea. 
It's really something the driver should figure out, and if we add it to 
the DT, it effectively becomes an ABI.

That said... I'm not sure how good of a job the driver could ever do, as 
it can't know the future scaling needs of the userspace at the time it 
is configuring the clock. And so, I'm not nacking this patch, but I 
don't feel very good about this patch...

The setting also affects all outputs (exluding venc), which may not be 
what the user wants. Then again, I think this setting is really only 
needed on OMAP2 & 3, which have only a single output. But that's the 
same with the current kconfig option, of course.

So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my 
opinion, and moving it to DT makes it a worse hack =). But I don't have 
any good suggestions either.

  Tomi
Adam Ford May 28, 2019, 3:09 p.m. UTC | #2
On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> Hi,
>
> On 10/05/2019 22:42, Adam Ford wrote:
> > Currently the source code is compiled using hard-coded values
> > from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
> > clock divider value to be moved to the device tree and be changed
> > without having to recompile the kernel.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> I understand why you want to do this, but I'm not sure it's a good idea.
> It's really something the driver should figure out, and if we add it to
> the DT, it effectively becomes an ABI.
>
> That said... I'm not sure how good of a job the driver could ever do, as
> it can't know the future scaling needs of the userspace at the time it
> is configuring the clock. And so, I'm not nacking this patch, but I
> don't feel very good about this patch...
>
> The setting also affects all outputs (exluding venc), which may not be
> what the user wants. Then again, I think this setting is really only
> needed on OMAP2 & 3, which have only a single output. But that's the
> same with the current kconfig option, of course.
>
> So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my
> opinion, and moving it to DT makes it a worse hack =). But I don't have
> any good suggestions either.

As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and
Torpedo) require this to be hard coded to 4 or it hangs during start.
This is the case for all versions 4.2+.  I haven't tested it with
older stuff.  Tony has a DM3730 Torpedo kit and reported the hanging
issue to me. I told him to set that value to 4 to make it not hang.
He asked that I move it to the DT to avoid custom kernels.  I agree
it's a hack, but if it's create a customized defconfig file for 4
boards or modify the device tree, it seems like the device tree
approach is less intrusive.

adam
>
>   Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
H. Nikolaus Schaller May 28, 2019, 3:20 p.m. UTC | #3
> Am 28.05.2019 um 17:09 schrieb Adam Ford <aford173@gmail.com>:
> 
> On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> 
>> Hi,
>> 
>> On 10/05/2019 22:42, Adam Ford wrote:
>>> Currently the source code is compiled using hard-coded values
>>> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
>>> clock divider value to be moved to the device tree and be changed
>>> without having to recompile the kernel.
>>> 
>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>> 
>> I understand why you want to do this, but I'm not sure it's a good idea.
>> It's really something the driver should figure out, and if we add it to
>> the DT, it effectively becomes an ABI.
>> 
>> That said... I'm not sure how good of a job the driver could ever do, as
>> it can't know the future scaling needs of the userspace at the time it
>> is configuring the clock. And so, I'm not nacking this patch, but I
>> don't feel very good about this patch...
>> 
>> The setting also affects all outputs (exluding venc), which may not be
>> what the user wants. Then again, I think this setting is really only
>> needed on OMAP2 & 3, which have only a single output. But that's the
>> same with the current kconfig option, of course.
>> 
>> So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my
>> opinion, and moving it to DT makes it a worse hack =). But I don't have
>> any good suggestions either.
> 
> As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and
> Torpedo) require this to be hard coded to 4 or it hangs during start.
> This is the case for all versions 4.2+.  I haven't tested it with
> older stuff.  Tony has a DM3730 Torpedo kit and reported the hanging
> issue to me. I told him to set that value to 4 to make it not hang.
> He asked that I move it to the DT to avoid custom kernels.  I agree
> it's a hack, but if it's create a customized defconfig file for 4
> boards or modify the device tree, it seems like the device tree
> approach is less intrusive.

Well, if this boards needs a factor 4 to be defined, it is IMHO
100 % correct to describe this in the DTS and nowhere else. Like
minimum and maximum voltage of a regulator which is also very board
specific.

Unless it can be figured out automatically. If it turns out later
that it can, I would assume the drivers can simply ignore the hint
in the DTS?

Just my 2cts without knowing details and having tested anything
on our DM37 boards.

BR,
Nikolaus
Tomi Valkeinen May 28, 2019, 3:53 p.m. UTC | #4
Hi,

On 28/05/2019 18:09, Adam Ford wrote:
> On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>> Hi,
>>
>> On 10/05/2019 22:42, Adam Ford wrote:
>>> Currently the source code is compiled using hard-coded values
>>> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
>>> clock divider value to be moved to the device tree and be changed
>>> without having to recompile the kernel.
>>>
>>> Signed-off-by: Adam Ford <aford173@gmail.com>
>>
>> I understand why you want to do this, but I'm not sure it's a good idea.
>> It's really something the driver should figure out, and if we add it to
>> the DT, it effectively becomes an ABI.
>>
>> That said... I'm not sure how good of a job the driver could ever do, as
>> it can't know the future scaling needs of the userspace at the time it
>> is configuring the clock. And so, I'm not nacking this patch, but I
>> don't feel very good about this patch...
>>
>> The setting also affects all outputs (exluding venc), which may not be
>> what the user wants. Then again, I think this setting is really only
>> needed on OMAP2 & 3, which have only a single output. But that's the
>> same with the current kconfig option, of course.
>>
>> So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my
>> opinion, and moving it to DT makes it a worse hack =). But I don't have
>> any good suggestions either.
> 
> As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and
> Torpedo) require this to be hard coded to 4 or it hangs during start.
> This is the case for all versions 4.2+.  I haven't tested it with
> older stuff.  Tony has a DM3730 Torpedo kit and reported the hanging
> issue to me. I told him to set that value to 4 to make it not hang.
> He asked that I move it to the DT to avoid custom kernels.  I agree
> it's a hack, but if it's create a customized defconfig file for 4
> boards or modify the device tree, it seems like the device tree
> approach is less intrusive.

Ok, well, I think that's a separate thing from its intended use. The 
point of the kconfig option is to ensure that the fclk/pclk ratio stays 
under a certain number to allow enough scaling range. It should never 
affect a basic non-scaling use case, unless you set it to a too high 
value, which prevents finding any pclk.

Has anyone debugged why the hang is happening?

If we can't fix the bug itself, rather than adding a DT option, we could 
change add a min_fck_per_pck field (as you do), keep the kconfig option, 
and set the min_fck_per_pck based on the kconfig option, _or_ in case of 
those affected SoCs, set the min_fck_per_pck even without the kconfig 
option.

  Tomi
Adam Ford May 31, 2019, 12:13 p.m. UTC | #5
On Tue, May 28, 2019 at 10:53 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> Hi,
>
> On 28/05/2019 18:09, Adam Ford wrote:
> > On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >>
> >> Hi,
> >>
> >> On 10/05/2019 22:42, Adam Ford wrote:
> >>> Currently the source code is compiled using hard-coded values
> >>> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
> >>> clock divider value to be moved to the device tree and be changed
> >>> without having to recompile the kernel.
> >>>
> >>> Signed-off-by: Adam Ford <aford173@gmail.com>
> >>
> >> I understand why you want to do this, but I'm not sure it's a good idea.
> >> It's really something the driver should figure out, and if we add it to
> >> the DT, it effectively becomes an ABI.
> >>
> >> That said... I'm not sure how good of a job the driver could ever do, as
> >> it can't know the future scaling needs of the userspace at the time it
> >> is configuring the clock. And so, I'm not nacking this patch, but I
> >> don't feel very good about this patch...
> >>
> >> The setting also affects all outputs (exluding venc), which may not be
> >> what the user wants. Then again, I think this setting is really only
> >> needed on OMAP2 & 3, which have only a single output. But that's the
> >> same with the current kconfig option, of course.
> >>
> >> So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my
> >> opinion, and moving it to DT makes it a worse hack =). But I don't have
> >> any good suggestions either.
> >
> > As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and
> > Torpedo) require this to be hard coded to 4 or it hangs during start.
> > This is the case for all versions 4.2+.  I haven't tested it with
> > older stuff.  Tony has a DM3730 Torpedo kit and reported the hanging
> > issue to me. I told him to set that value to 4 to make it not hang.
> > He asked that I move it to the DT to avoid custom kernels.  I agree
> > it's a hack, but if it's create a customized defconfig file for 4
> > boards or modify the device tree, it seems like the device tree
> > approach is less intrusive.
>
> Ok, well, I think that's a separate thing from its intended use. The
> point of the kconfig option is to ensure that the fclk/pclk ratio stays
> under a certain number to allow enough scaling range. It should never
> affect a basic non-scaling use case, unless you set it to a too high
> value, which prevents finding any pclk.
>
> Has anyone debugged why the hang is happening?

I tried debugging this years ago, and I was told to use the
CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.
>
> If we can't fix the bug itself, rather than adding a DT option, we could
> change add a min_fck_per_pck field (as you do), keep the kconfig option,
> and set the min_fck_per_pck based on the kconfig option, _or_ in case of
> those affected SoCs, set the min_fck_per_pck even without the kconfig
> option.

I am just curious if anyone else sees this.  If nobody is using this
hack, I wonder how much of the impact it will be.  I'm trying trying
to get my board to boot without hanging without creating a custom
defconfig.

adam
>
>   Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Rob Herring June 13, 2019, 8:22 p.m. UTC | #6
On Fri, May 10, 2019 at 02:42:29PM -0500, Adam Ford wrote:
> Currently the source code is compiled using hard-coded values
> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
> clock divider value to be moved to the device tree and be changed
> without having to recompile the kernel.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
> index cd02516a40b6..42449d07c47e 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
> +++ b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
> @@ -40,7 +40,7 @@ Required properties:
>  Optional properties:
>  - max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
>  			in bytes per second
> -
> +- min-fck-pck-ratio:  Make sure that DISPC FCK is at least n x PCK

Assuming this patch progresses, this needs a vendor prefix and please 
split bindings to separate patch.

>  
>  RFBI
>  ----
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 4043ecb38016..bf84a8487aae 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -751,7 +751,7 @@
>  			#size-cells = <1>;
>  			ranges;
>  
> -			dispc@48050400 {
> +			dispc: dispc@48050400 {

Unrelated change.

>  				compatible = "ti,omap3-dispc";
>  				reg = <0x48050400 0x400>;
>  				interrupts = <25>;
> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
> index f24ebf7f61dd..d0666edcdf2a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
> @@ -102,24 +102,6 @@ config OMAP2_DSS_DSI
>  
>  	  See http://www.mipi.org/ for DSI specifications.
>  
> -config OMAP2_DSS_MIN_FCK_PER_PCK
> -	int "Minimum FCK/PCK ratio (for scaling)"
> -	range 0 32
> -	default 0
> -	help
> -	  This can be used to adjust the minimum FCK/PCK ratio.
> -
> -	  With this you can make sure that DISPC FCK is at least
> -	  n x PCK. Video plane scaling requires higher FCK than
> -	  normally.
> -
> -	  If this is set to 0, there's no extra constraint on the
> -	  DISPC FCK. However, the FCK will at minimum be
> -	  2xPCK (if active matrix) or 3xPCK (if passive matrix).
> -
> -	  Max FCK is 173MHz, so this doesn't work if your PCK
> -	  is very high.
> -
>  config OMAP2_DSS_SLEEP_AFTER_VENC_RESET
>  	bool "Sleep 20ms after VENC reset"
>  	default y
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index ba82d916719c..09a130c53da2 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -198,6 +198,9 @@ struct dispc_device {
>  
>  	/* DISPC_CONTROL & DISPC_CONFIG lock*/
>  	spinlock_t control_lock;
> +
> +	/* Optional min-fck-pck-ratio */
> +	u32 min_fck_per_pck;
>  };
>  
>  enum omap_color_component {
> @@ -3683,15 +3686,8 @@ bool dispc_div_calc(struct dispc_device *dispc, unsigned long dispc_freq,
>  	unsigned long pck, lck;
>  	unsigned long lck_max;
>  	unsigned long pckd_hw_min, pckd_hw_max;
> -	unsigned int min_fck_per_pck;
>  	unsigned long fck;
>  
> -#ifdef CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK
> -	min_fck_per_pck = CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK;
> -#else
> -	min_fck_per_pck = 0;
> -#endif
> -
>  	pckd_hw_min = dispc->feat->min_pcd;
>  	pckd_hw_max = 255;
>  
> @@ -3723,7 +3719,7 @@ bool dispc_div_calc(struct dispc_device *dispc, unsigned long dispc_freq,
>  			else
>  				fck = lck;
>  
> -			if (fck < pck * min_fck_per_pck)
> +			if (fck < pck * dispc->min_fck_per_pck)
>  				continue;
>  
>  			if (func(lckd, pckd, lck, pck, data))
> @@ -4826,6 +4822,8 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
>  		}
>  	}
>  
> +	of_property_read_u32(np, "min-fck-pck-ratio", &dispc->min_fck_per_pck);
> +
>  	r = dispc_init_gamma_tables(dispc);
>  	if (r)
>  		goto err_free;
> -- 
> 2.17.1
>
Adam Ford Sept. 25, 2019, 8:51 p.m. UTC | #7
On Tue, May 28, 2019 at 10:53 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> Hi,
>
> On 28/05/2019 18:09, Adam Ford wrote:
> > On Tue, May 28, 2019 at 4:11 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >>
> >> Hi,
> >>
> >> On 10/05/2019 22:42, Adam Ford wrote:
> >>> Currently the source code is compiled using hard-coded values
> >>> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
> >>> clock divider value to be moved to the device tree and be changed
> >>> without having to recompile the kernel.
> >>>
> >>> Signed-off-by: Adam Ford <aford173@gmail.com>
> >>
> >> I understand why you want to do this, but I'm not sure it's a good idea.
> >> It's really something the driver should figure out, and if we add it to
> >> the DT, it effectively becomes an ABI.
> >>
> >> That said... I'm not sure how good of a job the driver could ever do, as
> >> it can't know the future scaling needs of the userspace at the time it
> >> is configuring the clock. And so, I'm not nacking this patch, but I
> >> don't feel very good about this patch...
> >>
> >> The setting also affects all outputs (exluding venc), which may not be
> >> what the user wants. Then again, I think this setting is really only
> >> needed on OMAP2 & 3, which have only a single output. But that's the
> >> same with the current kconfig option, of course.
> >>
> >> So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my
> >> opinion, and moving it to DT makes it a worse hack =). But I don't have
> >> any good suggestions either.
> >
> > As it stands the Logic PD OMAP35 and AM37/DM37 boards (SOM-LV and
> > Torpedo) require this to be hard coded to 4 or it hangs during start.
> > This is the case for all versions 4.2+.  I haven't tested it with
> > older stuff.  Tony has a DM3730 Torpedo kit and reported the hanging
> > issue to me. I told him to set that value to 4 to make it not hang.
> > He asked that I move it to the DT to avoid custom kernels.  I agree
> > it's a hack, but if it's create a customized defconfig file for 4
> > boards or modify the device tree, it seems like the device tree
> > approach is less intrusive.
>
> Ok, well, I think that's a separate thing from its intended use. The
> point of the kconfig option is to ensure that the fclk/pclk ratio stays
> under a certain number to allow enough scaling range. It should never
> affect a basic non-scaling use case, unless you set it to a too high
> value, which prevents finding any pclk.
>
> Has anyone debugged why the hang is happening?
I started to debug this, but I got distracted when I noticed the LCD
did't work at all on modern kernels.  I have that fixed now, so I can
go back to investigating this.

Working version:

[    7.999359] DISPC: dispc_runtime_get
[    7.999542] DSS: dss_restore_context
[    7.999542] DSS: context restored
[    7.999572] DISPC: dispc_runtime_put
[    7.999603] DISPC: dispc_save_context
[    7.999633] DISPC: context saved
[    7.999694] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[    7.999694] [drm] No driver support for vblank timestamp query.
[    8.025909] DSS: dss_save_context
[    8.025939] DSS: context saved
[    8.031951] DISPC: dispc_runtime_get
[    8.032043] DSS: dss_restore_context
[    8.032043] DSS: context restored
[    8.032073] DPI: dpi_set_timings
[    8.032104] DISPC: dispc_ovl_setup 0, pa 0x9e900000, pa_uv
0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
chan 0 repl 1
[    8.032135] DISPC: scrw 480, width 480
[    8.032135] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
[    8.032135] DISPC: 0,0 480x272 -> 480x272
[    8.032135] DISPC: dispc_enable_plane 0, 1
[    8.032165] DISPC: dispc_runtime_get
[    8.032196] DISPC: dispc_runtime_get
[    8.032196] DSS: set fck to 36000000
[    8.032257] DISPC: lck = 36000000 (1)
[    8.032257] DISPC: pck = 9000000 (4)
[    8.034240] DISPC: channel 0 xres 480 yres 272
[    8.034271] DISPC: pck 9000000
[    8.034271] DISPC: hsync_len 42 hfp 3 hbp 2 vsw 11 vfp 2 vbp 3
[    8.034271] DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1
de_level 1 sync_pclk_edge -1
[    8.034271] DISPC: hsync 17077Hz, vsync 59Hz
[    8.493347] DISPC: dispc_runtime_put
[    8.493438] Console: switching to colour frame buffer device 60x34
[    8.493774] DISPC: dispc_runtime_get
[    8.493835] DISPC: dispc_ovl_setup 0, pa 0x9e900000, pa_uv
0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
chan 0 repl 1
[    8.493896] DISPC: scrw 480, width 480
[    8.493896] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
[    8.493927] DISPC: 0,0 480x272 -> 480x272
[    8.493957] DISPC: dispc_enable_plane 0, 1
[    8.493988] DISPC: GO LCD
[    8.506774] DISPC: dispc_runtime_put
[    8.512298] omapdrm omapdrm.0: fb0: omapdrmdrmfb frame buffer device
[    8.516632] [drm] Initialized omapdrm 1.0.0 20110917 for omapdrm.0 on minor 0
[    8.581359] wlcore: WARNING Detected unconfigured mac address in
nvs, derive from fuse instead.
[    8.581359] wlcore: WARNING Your device performance is not optimized.
[    8.581390] wlcore: WARNING Please use the calibrator tool to
configure your device.
[    8.583862] wlcore: loaded
[    9.520355] DISPC: dispc_runtime_get
[    9.520446] DISPC: dispc_ovl_setup 0, pa 0x9e900000, pa_uv
0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
chan 0 repl 1
[    9.520477] DISPC: scrw 480, width 480
[    9.520507] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
[    9.520538] DISPC: 0,0 480x272 -> 480x272
[    9.520568] DISPC: dispc_enable_plane 0, 1
[    9.520599] DISPC: GO LCD
[    9.535400] DISPC: dispc_runtime_put


The Non-working version with the divisor set to 0:

[   10.719512] DSS: dss_runtime_get
[   10.723022] DSS: dss_restore_context
[   10.726623] DSS: OMAP DSS rev 2.0
[   10.730041] DSS: dss_runtime_put
[   10.733306] DSS: dss_save_context
[   10.736633] DSS: context saved
[   10.740417] DSS: dss_restore_context
[   10.744018] DSS: context restored
[   10.748046] DISPC: dispc_runtime_get
[   10.751770] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
[   10.758514] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
[   10.765289] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
[   10.772033] DISPC: dispc_restore_context
[   10.775970] DISPC: dispc_restore_gamma_tables()
[   10.780578] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
[   10.787322] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
[   10.794067] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
[   10.800842] omapdss_dispc 48050400.dispc: OMAP DISPC rev 3.0
[   10.806518] DISPC: dispc_runtime_put
[   10.810150] DISPC: dispc_save_context
[   10.813873] DISPC: context saved
[   10.817291] omapdss_dss 48050000.dss: bound 48050400.dispc (ops
hdmi5_configure [omapdss])
[   10.927215] DSS: dss_save_context
[   10.930725] DSS: context saved
[   11.097503] omapdrm omapdrm.0: DMM not available, disable DMM support
[   11.104248] omapdss_dss 48050000.dss: connect(NULL, 48050000.dss)
[   11.110473] omapdss_dss 48050000.dss: connect(48050000.dss, NULL)
[   11.116729] DISPC: dispc_runtime_get
[   11.120452] DSS: dss_restore_context
[   11.124053] DSS: context restored
[   11.127410] DISPC: dispc_runtime_put
[   11.131072] DISPC: dispc_save_context
[   11.134765] DISPC: context saved
[   11.138092] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   11.144775] [drm] No driver support for vblank timestamp query.
[   11.156372] DSS: dss_save_context
[   11.159729] DSS: context saved

** hang **

I noticed there doesn't seem to be the calculation for setting fck,
pck or any of the timings.  Are there any more debug options I can
enable?

I would prefer to fix the calculation so the display can work without
the defconfig or device tree modification, and then we dump this all
together.

>
> If we can't fix the bug itself, rather than adding a DT option, we could
> change add a min_fck_per_pck field (as you do), keep the kconfig option,
> and set the min_fck_per_pck based on the kconfig option, _or_ in case of
> those affected SoCs, set the min_fck_per_pck even without the kconfig
> option.
>
>   Tomi
>

thanks

adam
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Andrew Davis Sept. 25, 2019, 9:26 p.m. UTC | #8
On 5/28/19 4:11 AM, Tomi Valkeinen wrote:
> Hi,
> 
> On 10/05/2019 22:42, Adam Ford wrote:
>> Currently the source code is compiled using hard-coded values
>> from CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK.  This patch allows this
>> clock divider value to be moved to the device tree and be changed
>> without having to recompile the kernel.
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> I understand why you want to do this, but I'm not sure it's a good idea.
> It's really something the driver should figure out, and if we add it to
> the DT, it effectively becomes an ABI.
> 
> That said... I'm not sure how good of a job the driver could ever do, as
> it can't know the future scaling needs of the userspace at the time it
> is configuring the clock. And so, I'm not nacking this patch, but I
> don't feel very good about this patch...
> 
> The setting also affects all outputs (exluding venc), which may not be
> what the user wants. Then again, I think this setting is really only
> needed on OMAP2 & 3, which have only a single output. But that's the
> same with the current kconfig option, of course.
> 
> So, the current CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK is an ugly hack, in my
> opinion, and moving it to DT makes it a worse hack =). But I don't have
> any good suggestions either.
> 


Module param?

Andrew


>  Tomi
>
Tomi Valkeinen Sept. 26, 2019, 6:55 a.m. UTC | #9
On 25/09/2019 23:51, Adam Ford wrote:

>> Has anyone debugged why the hang is happening?
> I started to debug this, but I got distracted when I noticed the LCD
> did't work at all on modern kernels.  I have that fixed now, so I can
> go back to investigating this.
> 
> Working version:
> 
> [    7.999359] DISPC: dispc_runtime_get
> [    7.999542] DSS: dss_restore_context
> [    7.999542] DSS: context restored
> [    7.999572] DISPC: dispc_runtime_put
> [    7.999603] DISPC: dispc_save_context
> [    7.999633] DISPC: context saved
> [    7.999694] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [    7.999694] [drm] No driver support for vblank timestamp query.
> [    8.025909] DSS: dss_save_context
> [    8.025939] DSS: context saved
> [    8.031951] DISPC: dispc_runtime_get
> [    8.032043] DSS: dss_restore_context
> [    8.032043] DSS: context restored
> [    8.032073] DPI: dpi_set_timings
> [    8.032104] DISPC: dispc_ovl_setup 0, pa 0x9e900000, pa_uv
> 0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
> chan 0 repl 1
> [    8.032135] DISPC: scrw 480, width 480
> [    8.032135] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
> [    8.032135] DISPC: 0,0 480x272 -> 480x272
> [    8.032135] DISPC: dispc_enable_plane 0, 1
> [    8.032165] DISPC: dispc_runtime_get
> [    8.032196] DISPC: dispc_runtime_get
> [    8.032196] DSS: set fck to 36000000
> [    8.032257] DISPC: lck = 36000000 (1)
> [    8.032257] DISPC: pck = 9000000 (4)
> [    8.034240] DISPC: channel 0 xres 480 yres 272
> [    8.034271] DISPC: pck 9000000
> [    8.034271] DISPC: hsync_len 42 hfp 3 hbp 2 vsw 11 vfp 2 vbp 3
> [    8.034271] DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1
> de_level 1 sync_pclk_edge -1
> [    8.034271] DISPC: hsync 17077Hz, vsync 59Hz
> [    8.493347] DISPC: dispc_runtime_put
> [    8.493438] Console: switching to colour frame buffer device 60x34
> [    8.493774] DISPC: dispc_runtime_get
> [    8.493835] DISPC: dispc_ovl_setup 0, pa 0x9e900000, pa_uv
> 0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
> chan 0 repl 1
> [    8.493896] DISPC: scrw 480, width 480
> [    8.493896] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
> [    8.493927] DISPC: 0,0 480x272 -> 480x272
> [    8.493957] DISPC: dispc_enable_plane 0, 1
> [    8.493988] DISPC: GO LCD
> [    8.506774] DISPC: dispc_runtime_put
> [    8.512298] omapdrm omapdrm.0: fb0: omapdrmdrmfb frame buffer device
> [    8.516632] [drm] Initialized omapdrm 1.0.0 20110917 for omapdrm.0 on minor 0
> [    8.581359] wlcore: WARNING Detected unconfigured mac address in
> nvs, derive from fuse instead.
> [    8.581359] wlcore: WARNING Your device performance is not optimized.
> [    8.581390] wlcore: WARNING Please use the calibrator tool to
> configure your device.
> [    8.583862] wlcore: loaded
> [    9.520355] DISPC: dispc_runtime_get
> [    9.520446] DISPC: dispc_ovl_setup 0, pa 0x9e900000, pa_uv
> 0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
> chan 0 repl 1
> [    9.520477] DISPC: scrw 480, width 480
> [    9.520507] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
> [    9.520538] DISPC: 0,0 480x272 -> 480x272
> [    9.520568] DISPC: dispc_enable_plane 0, 1
> [    9.520599] DISPC: GO LCD
> [    9.535400] DISPC: dispc_runtime_put
> 
> 
> The Non-working version with the divisor set to 0:
> 
> [   10.719512] DSS: dss_runtime_get
> [   10.723022] DSS: dss_restore_context
> [   10.726623] DSS: OMAP DSS rev 2.0
> [   10.730041] DSS: dss_runtime_put
> [   10.733306] DSS: dss_save_context
> [   10.736633] DSS: context saved
> [   10.740417] DSS: dss_restore_context
> [   10.744018] DSS: context restored
> [   10.748046] DISPC: dispc_runtime_get
> [   10.751770] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
> [   10.758514] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
> [   10.765289] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
> [   10.772033] DISPC: dispc_restore_context
> [   10.775970] DISPC: dispc_restore_gamma_tables()
> [   10.780578] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
> [   10.787322] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
> [   10.794067] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
> [   10.800842] omapdss_dispc 48050400.dispc: OMAP DISPC rev 3.0
> [   10.806518] DISPC: dispc_runtime_put
> [   10.810150] DISPC: dispc_save_context
> [   10.813873] DISPC: context saved
> [   10.817291] omapdss_dss 48050000.dss: bound 48050400.dispc (ops
> hdmi5_configure [omapdss])
> [   10.927215] DSS: dss_save_context
> [   10.930725] DSS: context saved
> [   11.097503] omapdrm omapdrm.0: DMM not available, disable DMM support
> [   11.104248] omapdss_dss 48050000.dss: connect(NULL, 48050000.dss)
> [   11.110473] omapdss_dss 48050000.dss: connect(48050000.dss, NULL)
> [   11.116729] DISPC: dispc_runtime_get
> [   11.120452] DSS: dss_restore_context
> [   11.124053] DSS: context restored
> [   11.127410] DISPC: dispc_runtime_put
> [   11.131072] DISPC: dispc_save_context
> [   11.134765] DISPC: context saved
> [   11.138092] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [   11.144775] [drm] No driver support for vblank timestamp query.
> [   11.156372] DSS: dss_save_context
> [   11.159729] DSS: context saved
> 
> ** hang **
> 
> I noticed there doesn't seem to be the calculation for setting fck,
> pck or any of the timings.  Are there any more debug options I can
> enable?

The logs here look very different. The first one doesn't even show the 
DSS rev prints. Can you get full logs for both? And even better, if you 
can build omapdss as a kernel module, and load it after the boot, you 
won't have any "extra" going on at the same time.

And what is the hdmi5_configure there? I don't see anything in the 
driver that would print hdmi5_configure. And, of course, there's no 
hdmi5 on that platform. Hmm, ok... it's from component.c, using "%ps". 
Somehow that goes wrong. Which is a bit alarming, but perhaps a totally 
different issue.

The hang happens at an odd time. The last line shows that the driver has 
managed to do its work at suspend time. Afaics, the only thing the 
driver does after that is calling pinctrl_pm_select_sleep_state(). You 
could add a print after that to be sure that goes fine. But I suspect it 
does.

Which then hints that the hang is somewhere outside the driver, in 
omap_device perhaps?

You could try adding an extra call to dss_runtime_get(). Say, at the 
beginning of dss_probe_hardware(), do another dss_runtime_get(). That 
should force DSS to be always on (until reboot). runtime PM suspend 
related bugs should disappear.

  Tomi
Adam Ford Sept. 26, 2019, 2:12 p.m. UTC | #10
On Thu, Sep 26, 2019 at 1:55 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> On 25/09/2019 23:51, Adam Ford wrote:
>
> >> Has anyone debugged why the hang is happening?
> > I started to debug this, but I got distracted when I noticed the LCD
> > did't work at all on modern kernels.  I have that fixed now, so I can
> > go back to investigating this.
> >
> > Working version:
> >

> >
> > I noticed there doesn't seem to be the calculation for setting fck,
> > pck or any of the timings.  Are there any more debug options I can
> > enable?
>
> The logs here look very different. The first one doesn't even show the
> DSS rev prints. Can you get full logs for both? And even better, if you
> can build omapdss as a kernel module, and load it after the boot, you
> won't have any "extra" going on at the same time.

Since it's build as a module, I only dumped the stuff starting after
the modules are loading.  I can provide more if you want, but I am
trying to avoid excessive noise.

5.3.1 with drivers build as modules:

[    5.143615] random: udevd: uninitialized urandom read (16 bytes read)
[    5.153869] random: udevd: uninitialized urandom read (16 bytes read)
[    5.160522] random: udevd: uninitialized urandom read (16 bytes read)
[    5.187286] udevd[104]: specified group 'kvm' unknown
[    5.240875] udevd[105]: starting eudev-3.2.7
[    6.026672] DSS: set fck to 172800000
[    6.030487] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    6.084716] omap_ssi 48058000.ssi-controller: ssi controller 0
initialized (2 ports)!
[    6.093536] omap_ssi_port 4805a000.ssi-port: GPIO lookup for
consumer ti,ssi-cawake
[    6.101348] omap_ssi_port 4805a000.ssi-port: using device tree for
GPIO lookup
[    6.108642] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpios' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805a000[0]'
[    6.122131] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpio' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805a000[0]'
[    6.135559] omap_ssi_port 4805a000.ssi-port: using lookup tables
for GPIO lookup
[    6.143035] omap_ssi_port 4805a000.ssi-port: No GPIO consumer
ti,ssi-cawake found
[    6.150543] omap_ssi_port 4805a000.ssi-port: couldn't get cawake
gpio (err=-2)!
[    6.157958] omap_ssi_port: probe of 4805a000.ssi-port failed with error -2
[    6.164978] omap_ssi_port 4805b000.ssi-port: GPIO lookup for
consumer ti,ssi-cawake
[    6.172698] omap_ssi_port 4805b000.ssi-port: using device tree for
GPIO lookup
[    6.179992] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpios' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805b000[0]'
[    6.193481] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpio' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805b000[0]'
[    6.206909] omap_ssi_port 4805b000.ssi-port: using lookup tables
for GPIO lookup
[    6.214355] omap_ssi_port 4805b000.ssi-port: No GPIO consumer
ti,ssi-cawake found
[    6.221923] omap_ssi_port 4805b000.ssi-port: couldn't get cawake
gpio (err=-2)!
[    6.229278] omap_ssi_port: probe of 4805b000.ssi-port failed with error -2
[    6.265075] at24 2-0050: GPIO lookup for consumer wp
[    6.270080] at24 2-0050: using device tree for GPIO lookup
[    6.275756] of_get_named_gpiod_flags: can't parse 'wp-gpios'
property of node '/ocp@68000000/i2c@48060000/at24@50[0]'
[    6.286499] of_get_named_gpiod_flags: can't parse 'wp-gpio'
property of node '/ocp@68000000/i2c@48060000/at24@50[0]'
[    6.297119] at24 2-0050: using lookup tables for GPIO lookup
[    6.302856] at24 2-0050: No GPIO consumer wp found
[    6.324035] tsc2004 2-0048: GPIO lookup for consumer reset
[    6.329559] tsc2004 2-0048: using device tree for GPIO lookup
[    6.335571] of_get_named_gpiod_flags: can't parse 'reset-gpios'
property of node '/ocp@68000000/i2c@48060000/tsc2004@48[0]'
[    6.346862] of_get_named_gpiod_flags: can't parse 'reset-gpio'
property of node '/ocp@68000000/i2c@48060000/tsc2004@48[0]'
[    6.358001] tsc2004 2-0048: using lookup tables for GPIO lookup
[    6.363983] tsc2004 2-0048: No GPIO consumer reset found
[    6.417541] usbcore: registered new interface driver usbfs
[    6.423309] usbcore: registered new interface driver hub
[    6.428802] usbcore: registered new device driver usb
[    6.474761] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    6.523010] twl4030_keypad 48070000.i2c:twl@48:keypad: missing or
malformed property linux,keymap: -22
[    6.532531] twl4030_keypad 48070000.i2c:twl@48:keypad: Failed to build keymap
[    6.539764] twl4030_keypad: probe of 48070000.i2c:twl@48:keypad
failed with error -22
[    6.590362] ehci-omap: OMAP-EHCI Host Controller driver
[    6.596557] ehci-omap 48064800.ehci: EHCI Host Controller
[    6.602203] ehci-omap 48064800.ehci: new USB bus registered,
assigned bus number 1
[    6.646911] DSS: set fck to 172800000
[    6.650848] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    6.730804] at24 2-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
[    6.756164] input: twl4030_pwrbutton as
/devices/platform/68000000.ocp/48070000.i2c/i2c-0/0-0048/48070000.i2c:twl@48:pwrbutton/input/input2
[    6.778076] DSS: set fck to 172800000
[    6.782104] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    6.794891] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    6.892547] ohci-platform: OHCI generic platform driver
[    6.898437] ohci-platform 48064400.ohci: Generic Platform OHCI controller
[    6.905456] ohci-platform 48064400.ohci: new USB bus registered,
assigned bus number 2
[    6.954040] DSS: set fck to 172800000
[    6.957824] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    6.968170] input: TSC200X touchscreen as
/devices/platform/68000000.ocp/48060000.i2c/i2c-2/2-0048/input/input0
[    7.093811] DSS: set fck to 172800000
[    7.097625] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.224639] omap-mailbox 48094000.mailbox: omap mailbox rev 0x40
[    7.274658] twl_rtc 48070000.i2c:twl@48:rtc: Enabling TWL-RTC
[    7.292907] DSS: set fck to 172800000
[    7.297119] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.316192] twl_rtc 48070000.i2c:twl@48:rtc: registered as rtc0
[    7.431549] musb-hdrc musb-hdrc.0.auto: MUSB HDRC host driver
[    7.437469] musb-hdrc musb-hdrc.0.auto: new USB bus registered,
assigned bus number 3
[    7.450439] twl4030_usb 48070000.i2c:twl@48:twl4030-usb:
Initialized TWL4030 USB module
[    7.526092] usb usb3: New USB device found, idVendor=1d6b,
idProduct=0002, bcdDevice= 5.03
[    7.534576] usb usb3: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[    7.541931] usb usb3: Product: MUSB HDRC host driver
[    7.546936] usb usb3: Manufacturer: Linux
5.3.1-00003-g848fbc000e72-dirty musb-hcd
[    7.554595] usb usb3: SerialNumber: musb-hdrc.0.auto
[    7.590911] Driver for 1-wire Dallas network protocol.
[    7.640197] DSS: set fck to 172800000
[    7.644134] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.711212] omap_hdq 480b2000.1w: OMAP HDQ Hardware Rev 0.5. Driver
in Interrupt mode
[    7.722930] mc: Linux media interface: v0.10
[    7.809753] videodev: Linux video capture interface: v2.00
[    7.858215] ohci-platform 48064400.ohci: irq 92, io mem 0x48064400
[    7.875671] hub 3-0:1.0: USB hub found
[    7.890167] omap_wdt: OMAP Watchdog Timer Rev 0x31: initial timeout 60 sec
[    7.899566] DSS: set fck to 172800000
[    7.903533] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.915832] hub 3-0:1.0: 1 port detected
[    7.959960] w1_master_driver w1_bus_master1: Attaching one wire
slave 01.000000000000 crc 3d
[    8.080474] DSS: set fck to 172800000
[    8.084411] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    8.127838] power_supply bq27000-battery:
power_supply_get_battery_info currently only supports devicetree
[    8.162322] usb usb2: New USB device found, idVendor=1d6b,
idProduct=0001, bcdDevice= 5.03
[    8.170654] usb usb2: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[    8.178039] usb usb2: Product: Generic Platform OHCI controller
[    8.184051] usb usb2: Manufacturer: Linux
5.3.1-00003-g848fbc000e72-dirty ohci_hcd
[    8.191680] usb usb2: SerialNumber: 48064400.ohci
[    8.202484] ehci-omap 48064800.ehci: irq 93, io mem 0x48064800
[    8.245422] ehci-omap 48064800.ehci: USB 2.0 started, EHCI 1.00
[    8.283508] DSS: set fck to 172800000
[    8.287322] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    8.304565] omap3isp 480bc000.isp: ignoring dependency for device,
assuming no driver
[    8.312774] omap3isp 480bc000.isp: 480bc000.isp supply vdd-csiphy1
not found, using dummy regulator
[    8.322143] omap3isp 480bc000.isp: 480bc000.isp supply vdd-csiphy2
not found, using dummy regulator
[    8.331665] omap3isp 480bc000.isp: Revision 15.0 found
[    8.337585] omap-iommu 480bd400.mmu: 480bd400.mmu: version 1.1
[    8.343811] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
CCP2 was not initialized!
[    8.502380] DSS: set fck to 172800000
[    8.506195] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    8.580474] hub 2-0:1.0: USB hub found
[    8.611572] pwm-backlight backlight: GPIO lookup for consumer enable
[    8.617980] pwm-backlight backlight: using device tree for GPIO lookup
[    8.624694] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/backlight[0]' - status (0)
[    8.634368] gpio gpiochip4: Persistence not supported for GPIO 26
[    8.640502] no flags found for enable
[    8.644287] pwm-backlight backlight: backlight supply power not
found, using dummy regulator
[    8.661285] hub 2-0:1.0: 3 ports detected
[    8.674255] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/sysboot2[0]' - status (0)
[    8.684326] gpio gpiochip0: Persistence not supported for GPIO 2
[    8.690673] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/sysboot5[0]' - status (0)
[    8.700561] gpio gpiochip0: Persistence not supported for GPIO 7
[    8.706848] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/gpio1[0]' - status (0)
[    8.716491] gpio gpiochip5: Persistence not supported for GPIO 21
[    8.722839] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/gpio2[0]' - status (0)
[    8.732421] gpio gpiochip5: Persistence not supported for GPIO 18
[    8.738983] input: gpio_keys as /devices/platform/gpio_keys/input/input3
[    8.791473] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/leds/user0[0]' - status (0)
[    8.812042] usb usb1: New USB device found, idVendor=1d6b,
idProduct=0002, bcdDevice= 5.03
[    8.820373] usb usb1: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[    8.827728] usb usb1: Product: EHCI Host Controller
[    8.832672] usb usb1: Manufacturer: Linux
5.3.1-00003-g848fbc000e72-dirty ehci_hcd
[    8.840270] usb usb1: SerialNumber: 48064800.ehci
[    8.903961] no flags found for gpios
[    8.909759] of_get_named_gpiod_flags: can't parse
'ti,jack-det-gpio' property of node '/sound[0]'
[    8.919403] of_get_named_gpiod_flags: can't parse
'ti,hs_extmute_gpio' property of node
'/ocp@68000000/i2c@48070000/twl@48/audio/codec[0]'
[    8.933105] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/leds/led1[0]' - status (0)
[    8.942230] gpio gpiochip5: Persistence not supported for GPIO 20
[    8.948364] no flags found for gpios
[    8.952270] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/leds/led2[0]' - status (0)
[    8.961334] gpio gpiochip5: Persistence not supported for GPIO 19
[    8.967468] no flags found for gpios
[    9.002960] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
CSI2a was not initialized!
[    9.035003] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
CCDC was not initialized!
[    9.055084] hub 1-0:1.0: USB hub found
[    9.068878] panel-simple display: display supply power not found,
using dummy regulator
[    9.077239] panel-simple display: GPIO lookup for consumer enable
[    9.083465] panel-simple display: using device tree for GPIO lookup
[    9.089813] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/display[0]' - status (0)
[    9.099304] gpio gpiochip4: Persistence not supported for GPIO 27
[    9.115875] hub 1-0:1.0: 3 ports detected
[    9.120910] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
preview was not initialized!
[    9.164428] omap-twl4030 sound: twl4030-hifi <-> 49022000.mcbsp mapping ok
[    9.187957] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
resizer was not initialized!
[    9.244628] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
AEWB was not initialized!
[    9.253326] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
AF was not initialized!
[    9.261810] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
histogram was not initialized!
[    9.332031] pwm-backlight backlight: GPIO lookup for consumer enable
[    9.338439] pwm-backlight backlight: using device tree for GPIO lookup
[    9.345184] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/backlight[0]' - status (0)
[    9.354858] gpio gpiochip4: Persistence not supported for GPIO 26
[    9.361022] no flags found for enable
[    9.364776] pwm-backlight backlight: backlight supply power not
found, using dummy regulator
[    9.376739] panel-simple display: display supply power not found,
using dummy regulator
[    9.385040] panel-simple display: GPIO lookup for consumer enable
[    9.391204] panel-simple display: using device tree for GPIO lookup
[    9.397552] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/display[0]' - status (0)
[    9.407043] gpio gpiochip4: Persistence not supported for GPIO 27
[    9.413970] DSS: set fck to 172800000
[    9.417724] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    9.714416] DSS: dss_runtime_get
[    9.717773] DSS: dss_restore_context
[    9.721557] DSS: OMAP DSS rev 2.0
[    9.724884] DSS: dss_runtime_put
[    9.728149] DSS: dss_save_context
[    9.731506] DSS: context saved
[    9.735382] DSS: dss_restore_context
[    9.738983] DSS: context restored
[    9.743286] DISPC: dispc_runtime_get
[    9.746917] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
[    9.753753] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
[    9.760498] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
[    9.767242] DISPC: dispc_restore_context
[    9.771301] DISPC: dispc_restore_gamma_tables()
[    9.775909] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
[    9.782714] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
[    9.789428] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
[    9.796203] omapdss_dispc 48050400.dispc: OMAP DISPC rev 3.0
[    9.801940] DISPC: dispc_runtime_put
[    9.805541] DISPC: dispc_save_context
[    9.809265] DISPC: context saved
[    9.812744] omapdss_dss 48050000.dss: bound 48050400.dispc (ops
hdmi5_configure [omapdss])
[    9.839477] mousedev: PS/2 mouse device common for all mice
[   10.145874] cfg80211: Loading compiled-in X.509 certificates for
regulatory database
[   10.173217] DSS: dss_save_context
[   10.176666] DSS: context saved
[   10.317047] omapdrm omapdrm.0: DMM not available, disable DMM support
[   10.323730] omapdss_dss 48050000.dss: connect(NULL, 48050000.dss)
[   10.329864] omapdss_dss 48050000.dss: connect(48050000.dss, NULL)
[   10.336151] DISPC: dispc_runtime_get
[   10.339813] DSS: dss_restore_context
[   10.343475] DSS: context restored
[   10.346832] DISPC: dispc_runtime_put
[   10.350433] DISPC: dispc_save_context
[   10.354156] DISPC: context saved
[   10.357452] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   10.364135] [drm] No driver support for vblank timestamp query.
[   10.374847] DSS: dss_save_context
[   10.378265] DSS: context saved

Sorry for all the nosice, but the working splat with the divider set to 4:

Populating /dev using udev: [    4.766082] udevd[104]: starting version 3.2.7
[    4.829711] random: udevd: uninitialized urandom read (16 bytes read)
[    4.839935] random: udevd: uninitialized urandom read (16 bytes read)
[    4.847320] random: udevd: uninitialized urandom read (16 bytes read)
[    4.873870] udevd[104]: specified group 'kvm' unknown
[    4.926696] udevd[105]: starting eudev-3.2.7
[    5.715698] DSS: set fck to 172800000
[    5.719512] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    5.777435] omap_ssi 48058000.ssi-controller: ssi controller 0
initialized (2 ports)!
[    5.786315] omap_ssi_port 4805a000.ssi-port: GPIO lookup for
consumer ti,ssi-cawake
[    5.794128] omap_ssi_port 4805a000.ssi-port: using device tree for
GPIO lookup
[    5.801452] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpios' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805a000[0]'
[    5.814971] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpio' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805a000[0]'
[    5.828369] omap_ssi_port 4805a000.ssi-port: using lookup tables
for GPIO lookup
[    5.835845] omap_ssi_port 4805a000.ssi-port: No GPIO consumer
ti,ssi-cawake found
[    5.843414] omap_ssi_port 4805a000.ssi-port: couldn't get cawake
gpio (err=-2)!
[    5.850769] omap_ssi_port: probe of 4805a000.ssi-port failed with error -2
[    5.857788] omap_ssi_port 4805b000.ssi-port: GPIO lookup for
consumer ti,ssi-cawake
[    5.865539] omap_ssi_port 4805b000.ssi-port: using device tree for
GPIO lookup
[    5.872863] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpios' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805b000[0]'
[    5.886352] of_get_named_gpiod_flags: can't parse
'ti,ssi-cawake-gpio' property of node
'/ocp@68000000/ssi-controller@48058000/ssi-port@4805b000[0]'
[    5.899780] omap_ssi_port 4805b000.ssi-port: using lookup tables
for GPIO lookup
[    5.907257] omap_ssi_port 4805b000.ssi-port: No GPIO consumer
ti,ssi-cawake found
[    5.914794] omap_ssi_port 4805b000.ssi-port: couldn't get cawake
gpio (err=-2)!
[    5.922180] omap_ssi_port: probe of 4805b000.ssi-port failed with error -2
[    5.973175] at24 2-0050: GPIO lookup for consumer wp
[    5.978210] at24 2-0050: using device tree for GPIO lookup
[    5.983856] of_get_named_gpiod_flags: can't parse 'wp-gpios'
property of node '/ocp@68000000/i2c@48060000/at24@50[0]'
[    5.994567] of_get_named_gpiod_flags: can't parse 'wp-gpio'
property of node '/ocp@68000000/i2c@48060000/at24@50[0]'
[    6.005187] at24 2-0050: using lookup tables for GPIO lookup
[    6.010894] at24 2-0050: No GPIO consumer wp found
[    6.018280] tsc2004 2-0048: GPIO lookup for consumer reset
[    6.023956] tsc2004 2-0048: using device tree for GPIO lookup
[    6.029754] of_get_named_gpiod_flags: can't parse 'reset-gpios'
property of node '/ocp@68000000/i2c@48060000/tsc2004@48[0]'
[    6.041015] of_get_named_gpiod_flags: can't parse 'reset-gpio'
property of node '/ocp@68000000/i2c@48060000/tsc2004@48[0]'
[    6.052154] tsc2004 2-0048: using lookup tables for GPIO lookup
[    6.058105] tsc2004 2-0048: No GPIO consumer reset found
[    6.159973] usbcore: registered new interface driver usbfs
[    6.165771] usbcore: registered new interface driver hub
[    6.171325] usbcore: registered new device driver usb
[    6.211181] twl4030_keypad 48070000.i2c:twl@48:keypad: missing or
malformed property linux,keymap: -22
[    6.220550] twl4030_keypad 48070000.i2c:twl@48:keypad: Failed to build keymap
[    6.227844] twl4030_keypad: probe of 48070000.i2c:twl@48:keypad
failed with error -22
[    6.242553] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[    6.265563] ohci-platform: OHCI generic platform driver
[    6.271759] ohci-platform 48064400.ohci: Generic Platform OHCI controller
[    6.278625] ohci-platform 48064400.ohci: new USB bus registered,
assigned bus number 1
[    6.420623] input: twl4030_pwrbutton as
/devices/platform/68000000.ocp/48070000.i2c/i2c-0/0-0048/48070000.i2c:twl@48:pwrbutton/input/input2
[    6.438446] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    6.445220] Warning! ehci_hcd should always be loaded before
uhci_hcd and ohci_hcd, not after
[    6.491607] at24 2-0050: 8192 byte 24c64 EEPROM, writable, 1 bytes/write
[    6.506927] DSS: set fck to 172800000
[    6.510711] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    6.608886] ehci-omap: OMAP-EHCI Host Controller driver
[    6.614868] ehci-omap 48064800.ehci: EHCI Host Controller
[    6.620330] ehci-omap 48064800.ehci: new USB bus registered,
assigned bus number 2
[    6.647247] input: TSC200X touchscreen as
/devices/platform/68000000.ocp/48060000.i2c/i2c-2/2-0048/input/input0
[    6.659362] DSS: set fck to 172800000
[    6.663299] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    6.804473] DSS: set fck to 172800000
[    6.808288] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    6.890747] ohci-platform 48064400.ohci: irq 92, io mem 0x48064400
[    6.947784] omap-mailbox 48094000.mailbox: omap mailbox rev 0x40
[    7.014312] DSS: set fck to 172800000
[    7.018127] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.029022] twl_rtc 48070000.i2c:twl@48:rtc: Enabling TWL-RTC
[    7.096252] twl_rtc 48070000.i2c:twl@48:rtc: registered as rtc0
[    7.154327] DSS: set fck to 172800000
[    7.158111] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.168792] usb usb1: New USB device found, idVendor=1d6b,
idProduct=0001, bcdDevice= 5.03
[    7.177246] usb usb1: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[    7.184570] usb usb1: Product: Generic Platform OHCI controller
[    7.190521] usb usb1: Manufacturer: Linux
5.3.1-00004-g468b8eee984c-dirty ohci_hcd
[    7.198181] usb usb1: SerialNumber: 48064400.ohci
[    7.204467] musb-hdrc musb-hdrc.0.auto: MUSB HDRC host driver
[    7.235412] Driver for 1-wire Dallas network protocol.
[    7.282104] omap_hdq 480b2000.1w: OMAP HDQ Hardware Rev 0.5. Driver
in Interrupt mode
[    7.322814] ehci-omap 48064800.ehci: irq 93, io mem 0x48064800
[    7.330871] twl4030_usb 48070000.i2c:twl@48:twl4030-usb:
Initialized TWL4030 USB module
[    7.342529] DSS: set fck to 172800000
[    7.346435] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.375427] ehci-omap 48064800.ehci: USB 2.0 started, EHCI 1.00
[    7.429779] hub 1-0:1.0: USB hub found
[    7.433898] hub 1-0:1.0: 3 ports detected
[    7.458923] DSS: set fck to 172800000
[    7.462921] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.472778] musb-hdrc musb-hdrc.0.auto: new USB bus registered,
assigned bus number 3
[    7.510559] w1_master_driver w1_bus_master1: Attaching one wire
slave 01.000000000000 crc 3d
[    7.526550] mc: Linux media interface: v0.10
[    7.590454] usb usb3: New USB device found, idVendor=1d6b,
idProduct=0002, bcdDevice= 5.03
[    7.598937] usb usb3: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[    7.606231] usb usb3: Product: MUSB HDRC host driver
[    7.611297] usb usb3: Manufacturer: Linux
5.3.1-00004-g468b8eee984c-dirty musb-hcd
[    7.618896] usb usb3: SerialNumber: musb-hdrc.0.auto
[    7.691284] DSS: set fck to 172800000
[    7.695190] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    7.738800] power_supply bq27000-battery:
power_supply_get_battery_info currently only supports devicetree
[    7.772583] videodev: Linux video capture interface: v2.00
[    7.810089] omap_wdt: OMAP Watchdog Timer Rev 0x31: initial timeout 60 sec
[    7.908386] DSS: set fck to 172800000
[    7.912292] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    8.029205] hub 3-0:1.0: USB hub found
[    8.077514] hub 3-0:1.0: 1 port detected
[    8.129760] usb usb2: New USB device found, idVendor=1d6b,
idProduct=0002, bcdDevice= 5.03
[    8.138305] usb usb2: New USB device strings: Mfr=3, Product=2,
SerialNumber=1
[    8.145629] usb usb2: Product: EHCI Host Controller
[    8.150543] usb usb2: Manufacturer: Linux
5.3.1-00004-g468b8eee984c-dirty ehci_hcd
[    8.158172] usb usb2: SerialNumber: 48064800.ehci
[    8.201690] omap3isp 480bc000.isp: ignoring dependency for device,
assuming no driver
[    8.209808] omap3isp 480bc000.isp: 480bc000.isp supply vdd-csiphy1
not found, using dummy regulator
[    8.219207] omap3isp 480bc000.isp: 480bc000.isp supply vdd-csiphy2
not found, using dummy regulator
[    8.228729] omap3isp 480bc000.isp: Revision 15.0 found
[    8.234710] omap-iommu 480bd400.mmu: 480bd400.mmu: version 1.1
[    8.240844] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
CCP2 was not initialized!
[    8.268341] DSS: set fck to 172800000
[    8.272338] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    8.372863] pwm-backlight backlight: GPIO lookup for consumer enable
[    8.379272] pwm-backlight backlight: using device tree for GPIO lookup
[    8.386047] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/backlight[0]' - status (0)
[    8.395751] gpio gpiochip4: Persistence not supported for GPIO 26
[    8.401916] no flags found for enable
[    8.405639] pwm-backlight backlight: backlight supply power not
found, using dummy regulator
[    8.437988] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/sysboot2[0]' - status (0)
[    8.448120] gpio gpiochip0: Persistence not supported for GPIO 2
[    8.454620] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/sysboot5[0]' - status (0)
[    8.464477] gpio gpiochip0: Persistence not supported for GPIO 7
[    8.470733] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/gpio1[0]' - status (0)
[    8.480346] gpio gpiochip5: Persistence not supported for GPIO 21
[    8.486694] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/gpio_keys/gpio2[0]' - status (0)
[    8.496307] gpio gpiochip5: Persistence not supported for GPIO 18
[    8.502868] input: gpio_keys as /devices/platform/gpio_keys/input/input3
[    8.555450] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/leds/user0[0]' - status (0)
[    8.614593] of_get_named_gpiod_flags: can't parse
'ti,jack-det-gpio' property of node '/sound[0]'
[    8.624176] of_get_named_gpiod_flags: can't parse
'ti,hs_extmute_gpio' property of node
'/ocp@68000000/i2c@48070000/twl@48/audio/codec[0]'
[    8.655914] hub 2-0:1.0: USB hub found
[    8.671874] hub 2-0:1.0: 3 ports detected
[    8.738494] no flags found for gpios
[    8.746582] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/leds/led1[0]' - status (0)
[    8.755798] gpio gpiochip5: Persistence not supported for GPIO 20
[    8.761993] no flags found for gpios
[    8.765838] of_get_named_gpiod_flags: parsed 'gpios' property of
node '/leds/led2[0]' - status (0)
[    8.774902] gpio gpiochip5: Persistence not supported for GPIO 19
[    8.781066] no flags found for gpios
[    8.815582] panel-simple display: display supply power not found,
using dummy regulator
[    8.823944] panel-simple display: GPIO lookup for consumer enable
[    8.830078] panel-simple display: using device tree for GPIO lookup
[    8.836517] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/display[0]' - status (0)
[    8.846038] gpio gpiochip4: Persistence not supported for GPIO 27
[    8.863037] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
CSI2a was not initialized!
[    8.880096] omap-twl4030 sound: twl4030-hifi <-> 49022000.mcbsp mapping ok
[    8.909973] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
CCDC was not initialized!
[    8.955505] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
preview was not initialized!
[    8.996673] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
resizer was not initialized!
[    9.024017] pwm-backlight backlight: GPIO lookup for consumer enable
[    9.030426] pwm-backlight backlight: using device tree for GPIO lookup
[    9.037170] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/backlight[0]' - status (0)
[    9.046813] gpio gpiochip4: Persistence not supported for GPIO 26
[    9.052978] no flags found for enable
[    9.056701] pwm-backlight backlight: backlight supply power not
found, using dummy regulator
[    9.068450] panel-simple display: display supply power not found,
using dummy regulator
[    9.076690] panel-simple display: GPIO lookup for consumer enable
[    9.082855] panel-simple display: using device tree for GPIO lookup
[    9.089202] of_get_named_gpiod_flags: parsed 'enable-gpios'
property of node '/display[0]' - status (0)
[    9.098693] gpio gpiochip4: Persistence not supported for GPIO 27
[    9.105438] DSS: set fck to 172800000
[    9.109191] omapdss_dss 48050000.dss: 48050000.dss supply
vdda_video not found, using dummy regulator
[    9.139343] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
AEWB was not initialized!
[    9.148101] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
AF was not initialized!
[    9.156555] omap3isp 480bc000.isp: Entity type for entity OMAP3 ISP
histogram was not initialized!
[    9.296691] mousedev: PS/2 mouse device common for all mice
[    9.480438] DSS: dss_runtime_get
[    9.483886] DSS: dss_restore_context
[    9.487487] DSS: OMAP DSS rev 2.0
[    9.490814] DSS: dss_runtime_put
[    9.494140] DSS: dss_save_context
[    9.497467] DSS: context saved
[    9.501251] DSS: dss_restore_context
[    9.504852] DSS: context restored
[    9.508941] DISPC: dispc_runtime_get
[    9.512725] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
[    9.519470] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
[    9.526275] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
[    9.533020] DISPC: dispc_restore_context
[    9.536987] DISPC: dispc_restore_gamma_tables()
[    9.541564] DISPC: fifo(0) threshold (bytes), old 960/1023, new 960/1023
[    9.548309] DISPC: fifo(1) threshold (bytes), old 960/1023, new 960/1023
[    9.555053] DISPC: fifo(2) threshold (bytes), old 960/1023, new 960/1023
[    9.561828] omapdss_dispc 48050400.dispc: OMAP DISPC rev 3.0
[    9.567504] DISPC: dispc_runtime_put
[    9.571136] DISPC: dispc_save_context
[    9.574829] DISPC: context saved
[    9.578247] omapdss_dss 48050000.dss: bound 48050400.dispc (ops
hdmi5_configure [omapdss])
[    9.709533] cfg80211: Loading compiled-in X.509 certificates for
regulatory database
[    9.781860] DSS: dss_save_context
[    9.785278] DSS: context saved
[    9.967437] omapdrm omapdrm.0: DMM not available, disable DMM support
[    9.974121] omapdss_dss 48050000.dss: connect(NULL, 48050000.dss)
[    9.980255] omapdss_dss 48050000.dss: connect(48050000.dss, NULL)
[    9.986541] DISPC: dispc_runtime_get
[    9.990203] DSS: dss_restore_context
[    9.993865] DSS: context restored
[    9.997222] DISPC: dispc_runtime_put
[   10.000793] DISPC: dispc_save_context
[   10.004547] DISPC: context saved
[   10.007843] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[   10.014526] [drm] No driver support for vblank timestamp query.
[   10.022430] DSS: dss_save_context
[   10.025787] DSS: context saved
[   10.059051] cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
[   10.076721] DISPC: dispc_runtime_get
[   10.076812] DSS: dss_restore_context
[   10.076812] DSS: context restored
[   10.076873] DPI: dpi_set_timings
[   10.076904] DISPC: dispc_ovl_setup 0, pa 0x8e900000, pa_uv
0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
chan 0 repl 1
[   10.076904] DISPC: scrw 480, width 480
[   10.076904] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
[   10.076934] DISPC: 0,0 480x272 -> 480x272
[   10.076934] DISPC: dispc_enable_plane 0, 1
[   10.076934] DISPC: dispc_runtime_get
[   10.076995] DISPC: dispc_runtime_get
[   10.076995] DSS: set fck to 36000000
[   10.077026] DISPC: lck = 36000000 (1)
[   10.077026] DISPC: pck = 9000000 (4)
[   10.079132] DISPC: channel 0 xres 480 yres 272
[   10.079132] DISPC: pck 9000000
[   10.079132] DISPC: hsync_len 42 hfp 3 hbp 2 vsw 11 vfp 2 vbp 3
[   10.079162] DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1
de_level 1 sync_pclk_edge -1
[   10.079162] DISPC: hsync 17077Hz, vsync 59Hz
[   10.564025] DISPC: dispc_runtime_put
[   10.564147] Console: switching to colour frame buffer device 60x34
[   10.564514] DISPC: dispc_runtime_get
[   10.564575] DISPC: dispc_ovl_setup 0, pa 0x8e900000, pa_uv
0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
chan 0 repl 1
[   10.564605] DISPC: scrw 480, width 480
[   10.564636] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
[   10.564666] DISPC: 0,0 480x272 -> 480x272
[   10.564666] DISPC: dispc_enable_plane 0, 1
[   10.564697] DISPC: GO LCD
[   10.568481] DISPC: dispc_runtime_put
[   10.718139] omapdrm omapdrm.0: fb0: omapdrmdrmfb frame buffer device
[   10.726226] [drm] Initialized omapdrm 1.0.0 20110917 for omapdrm.0 on minor 0
done
Initializing random number generator... [   10.828277] urandom_read: 1
callbacks suppressed
[   10.828277] random: dd: uninitialized urandom read (512 bytes read)
done.
Starting system message bus: [   10.896789] random: dbus-uuidgen:
uninitialized urandom read (12 bytes read)
[   10.904510] random: dbus-uuidgen: uninitialized urandom read (8 bytes read)
done
Starting network: OK

Welcome to Buildroot
buildroot login: [   11.284576] wlcore: WARNING Detected unconfigured
mac address in nvs, derive from fuse instead.
[   11.293518] wlcore: WARNING Your device performance is not optimized.
[   11.299987] wlcore: WARNING Please use the calibrator tool to
configure your device.
[   11.313751] wlcore: loaded
[   11.761871] DISPC: dispc_runtime_get
[   11.765563] DISPC: dispc_ovl_setup 0, pa 0x8e900000, pa_uv
0x00000000, sw 480, 0,0, 480x272 -> 480x272, cmode 34325258, rot 1,
chan 0 repl 1
[   11.778472] DISPC: scrw 480, width 480
[   11.782348] DISPC: offset0 0, offset1 0, row_inc 1, pix_inc 1
[   11.788177] DISPC: 0,0 480x272 -> 480x272
[   11.792297] DISPC: dispc_enable_plane 0, 1
[   11.796447] DISPC: GO LCD
[   11.803985] DISPC: dispc_runtime_put

>
> And what is the hdmi5_configure there? I don't see anything in the
> driver that would print hdmi5_configure. And, of course, there's no
> hdmi5 on that platform. Hmm, ok... it's from component.c, using "%ps".
> Somehow that goes wrong. Which is a bit alarming, but perhaps a totally
> different issue.

I'll try to take a look later.  For Logic PD distributions, we create
a custom defconfig with all those drivers removed, so I'm not worked
up about it, but it would be nice to not call drivers that don't
exist.

>
> The hang happens at an odd time. The last line shows that the driver has
> managed to do its work at suspend time. Afaics, the only thing the
> driver does after that is calling pinctrl_pm_select_sleep_state(). You
> could add a print after that to be sure that goes fine. But I suspect it
> does.
>
> Which then hints that the hang is somewhere outside the driver, in
> omap_device perhaps?

Thanks for reviewing this.  I've been coping for a while by manually
changing the config option, but with 5.4 being the expected next LTS,
I was hoping to address this so I don't have to keep working around
it.

>
> You could try adding an extra call to dss_runtime_get(). Say, at the
> beginning of dss_probe_hardware(), do another dss_runtime_get(). That
> should force DSS to be always on (until reboot). runtime PM suspend
> related bugs should disappear.

I'll send out a second e-mail with some of your suggestions, but I
don't want to litter this e-mail with too many logs.

adam
>
>   Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tomi Valkeinen Sept. 27, 2019, 6:21 a.m. UTC | #11
On 26/09/2019 17:12, Adam Ford wrote:

>> And what is the hdmi5_configure there? I don't see anything in the
>> driver that would print hdmi5_configure. And, of course, there's no
>> hdmi5 on that platform. Hmm, ok... it's from component.c, using "%ps".
>> Somehow that goes wrong. Which is a bit alarming, but perhaps a totally
>> different issue.
> 
> I'll try to take a look later.  For Logic PD distributions, we create
> a custom defconfig with all those drivers removed, so I'm not worked
> up about it, but it would be nice to not call drivers that don't
> exist.

So you have CONFIG_OMAP5_DSS_HDMI=n? Then it's even more disturbing, as 
there's no way the string "hdmi5_configure" can be in the kernel image...

Maybe it's nothing, but... It's just so odd.

  Tomi
Tomi Valkeinen Sept. 27, 2019, 7:55 a.m. UTC | #12
(dropping folks who're probably not interested...)

On 26/09/2019 17:12, Adam Ford wrote:
> On Thu, Sep 26, 2019 at 1:55 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>> On 25/09/2019 23:51, Adam Ford wrote:
>>
>>>> Has anyone debugged why the hang is happening?
>>> I started to debug this, but I got distracted when I noticed the LCD
>>> did't work at all on modern kernels.  I have that fixed now, so I can
>>> go back to investigating this.

I dont' have the same board, but I was testing with omap3 beagle xm. I 
can reproduce rather similar issue, although I don't get a hang but 
instead sync lost and underflow flood (which makes the device unusable).

It looks like a bug in omap clock handling.

DSS uses dss1_alwon_fck_3430es2 as fclk. dss1_alwon_fck_3430es2 comes 
from dpll4_ck, and there's a divider after the PLL, dpll4_m4_ck.

When the DSS driver sets dss1_alwon_fck_3430es2 rate to 27000000 or 
27870967, which can be created with m4 dividers 32 and 31, it looks like 
the divider goes to bypass, or to a very small value. DSS gets a very 
high clock rate and breaks down.

  Tomi
Adam Ford Sept. 27, 2019, 12:13 p.m. UTC | #13
On Fri, Sep 27, 2019 at 1:22 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> On 26/09/2019 17:12, Adam Ford wrote:
>
> >> And what is the hdmi5_configure there? I don't see anything in the
> >> driver that would print hdmi5_configure. And, of course, there's no
> >> hdmi5 on that platform. Hmm, ok... it's from component.c, using "%ps".
> >> Somehow that goes wrong. Which is a bit alarming, but perhaps a totally
> >> different issue.
> >
> > I'll try to take a look later.  For Logic PD distributions, we create
> > a custom defconfig with all those drivers removed, so I'm not worked
> > up about it, but it would be nice to not call drivers that don't
> > exist.
>
> So you have CONFIG_OMAP5_DSS_HDMI=n? Then it's even more disturbing, as
> there's no way the string "hdmi5_configure" can be in the kernel image...

For the logs and problems I am showing in this thread, I am using a
stock omap2plus_defconfig which has it enabled.  I was only trying to
state that I am not worried about the omap5 hdmi stuff, because when I
do a custom distribution for Logic PD, I remove those config options
to make the issue go away.
>
> Maybe it's nothing, but... It's just so odd.

I don't think we need to worry about it now.  Ideally, it would be
nice to have the drivers recognize they are not needed and or setup
the Kconfig options to make these drivers dependent on the platforms
that support it so unselecting OMAP5 could make the omap5 options
disappear.

Sorry if I accidentally threw in a distraction or confusion.

adam
>
>   Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Adam Ford Sept. 27, 2019, 12:33 p.m. UTC | #14
On Fri, Sep 27, 2019 at 2:55 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> (dropping folks who're probably not interested...)
>
> On 26/09/2019 17:12, Adam Ford wrote:
> > On Thu, Sep 26, 2019 at 1:55 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >>
> >> On 25/09/2019 23:51, Adam Ford wrote:
> >>
> >>>> Has anyone debugged why the hang is happening?
> >>> I started to debug this, but I got distracted when I noticed the LCD
> >>> did't work at all on modern kernels.  I have that fixed now, so I can
> >>> go back to investigating this.
>
> I dont' have the same board, but I was testing with omap3 beagle xm. I
> can reproduce rather similar issue, although I don't get a hang but
> instead sync lost and underflow flood (which makes the device unusable).
>
> It looks like a bug in omap clock handling.
>
> DSS uses dss1_alwon_fck_3430es2 as fclk. dss1_alwon_fck_3430es2 comes
> from dpll4_ck, and there's a divider after the PLL, dpll4_m4_ck.
>
> When the DSS driver sets dss1_alwon_fck_3430es2 rate to 27000000 or
> 27870967, which can be created with m4 dividers 32 and 31, it looks like
> the divider goes to bypass, or to a very small value. DSS gets a very
> high clock rate and breaks down.

Is there anything I can do to help troubleshoot this?  I could insert
a hack that checks if we're omap3 and if so make the divider equal to
4, but that seems like just a hack.
I can run more tests or insert code somewhere if you want.

adam
>
>   Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tomi Valkeinen Sept. 27, 2019, 1:45 p.m. UTC | #15
On 27/09/2019 15:13, Adam Ford wrote:
> On Fri, Sep 27, 2019 at 1:22 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>> On 26/09/2019 17:12, Adam Ford wrote:
>>
>>>> And what is the hdmi5_configure there? I don't see anything in the
>>>> driver that would print hdmi5_configure. And, of course, there's no
>>>> hdmi5 on that platform. Hmm, ok... it's from component.c, using "%ps".
>>>> Somehow that goes wrong. Which is a bit alarming, but perhaps a totally
>>>> different issue.
>>>
>>> I'll try to take a look later.  For Logic PD distributions, we create
>>> a custom defconfig with all those drivers removed, so I'm not worked
>>> up about it, but it would be nice to not call drivers that don't
>>> exist.
>>
>> So you have CONFIG_OMAP5_DSS_HDMI=n? Then it's even more disturbing, as
>> there's no way the string "hdmi5_configure" can be in the kernel image...
> 
> For the logs and problems I am showing in this thread, I am using a
> stock omap2plus_defconfig which has it enabled.  I was only trying to
> state that I am not worried about the omap5 hdmi stuff, because when I
> do a custom distribution for Logic PD, I remove those config options
> to make the issue go away.
>>
>> Maybe it's nothing, but... It's just so odd.
> 
> I don't think we need to worry about it now.  Ideally, it would be
> nice to have the drivers recognize they are not needed and or setup
> the Kconfig options to make these drivers dependent on the platforms
> that support it so unselecting OMAP5 could make the omap5 options
> disappear.

My point is that something is bugging there. It should not print 
hdmi5_configure, regardless of setup or platform. If I'm not mistaken, 
it should print "dispc_component_ops". But somehow it gets a wrong 
symbol string.

But yes, I'm 99.9% sure it's not related, so let's ignore it here =).

  Tomi
Tomi Valkeinen Sept. 27, 2019, 1:47 p.m. UTC | #16
On 27/09/2019 15:33, Adam Ford wrote:

>> It looks like a bug in omap clock handling.
>>
>> DSS uses dss1_alwon_fck_3430es2 as fclk. dss1_alwon_fck_3430es2 comes
>> from dpll4_ck, and there's a divider after the PLL, dpll4_m4_ck.
>>
>> When the DSS driver sets dss1_alwon_fck_3430es2 rate to 27000000 or
>> 27870967, which can be created with m4 dividers 32 and 31, it looks like
>> the divider goes to bypass, or to a very small value. DSS gets a very
>> high clock rate and breaks down.
> 
> Is there anything I can do to help troubleshoot this?  I could insert
> a hack that checks if we're omap3 and if so make the divider equal to
> 4, but that seems like just a hack.
> I can run more tests or insert code somewhere if you want.

I think it's up to someone who's knowledgeable in omap clock framework. 
I'm kind of hoping that Tero or Tony would be willing to debug =). I can 
try to find time to debug the omap clk framework, but I'll be going on 
blindly there.

  Tomi
Tero Kristo Sept. 27, 2019, 3:37 p.m. UTC | #17
On 27/09/2019 16:47, Tomi Valkeinen wrote:
> On 27/09/2019 15:33, Adam Ford wrote:
> 
>>> It looks like a bug in omap clock handling.
>>>
>>> DSS uses dss1_alwon_fck_3430es2 as fclk. dss1_alwon_fck_3430es2 comes
>>> from dpll4_ck, and there's a divider after the PLL, dpll4_m4_ck.
>>>
>>> When the DSS driver sets dss1_alwon_fck_3430es2 rate to 27000000 or
>>> 27870967, which can be created with m4 dividers 32 and 31, it looks like
>>> the divider goes to bypass, or to a very small value. DSS gets a very
>>> high clock rate and breaks down.
>>
>> Is there anything I can do to help troubleshoot this?  I could insert
>> a hack that checks if we're omap3 and if so make the divider equal to
>> 4, but that seems like just a hack.
>> I can run more tests or insert code somewhere if you want.
> 
> I think it's up to someone who's knowledgeable in omap clock framework. 
> I'm kind of hoping that Tero or Tony would be willing to debug =). I can 
> try to find time to debug the omap clk framework, but I'll be going on 
> blindly there.

If you can provide details about what clock framework / driver does 
wrong (sample clk_set_xyz call sequence, expected results via 
clk_get_xyz, and what fails), I can take a look at it. Just reporting 
arbitrary display driver issues I won't be able to debug at all (I don't 
have access to any of the displays, nor do I want to waste time 
debugging them without absolutely no knowledge whatsoever.)

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tomi Valkeinen Sept. 27, 2019, 3:47 p.m. UTC | #18
On 27/09/2019 18:37, Tero Kristo wrote:

> If you can provide details about what clock framework / driver does 
> wrong (sample clk_set_xyz call sequence, expected results via 
> clk_get_xyz, and what fails), I can take a look at it. Just reporting 
> arbitrary display driver issues I won't be able to debug at all (I don't 
> have access to any of the displays, nor do I want to waste time 
> debugging them without absolutely no knowledge whatsoever.)

I used your hack patches to allow changing rates via debugfs. And set 
dss1_alwon_fck_3430es2 to 27000000 or 27870967. The end result was that 
DSS gets some very high clock from dss1_alwon_fck_3430es2, as the frame 
rate jumps to many hundreds fps.

So, these numbers are not real, but to give the idea what I saw. Running 
first with 50 MHz, I can see, say, 40 fps. Then I set the clock to 30 
MHz, and fps dropped to, say, 30fps, as expected with lower clock. Then 
I set the clock to 27MHz (or the other one), expecting a bit lower fps, 
but instead I saw hundreds of fps.

I don't know if there's any other way to observe the wrong clock rate 
but have the dss enabled and running kmstest or similar. I can help you 
set that up next week, should be trivial. You don't need a display for that.

  Tomi
Adam Ford Sept. 30, 2019, 12:41 p.m. UTC | #19
On Mon, Sep 30, 2019 at 3:53 AM Tero Kristo <t-kristo@ti.com> wrote:
>
> On 30/09/2019 09:45, Tomi Valkeinen wrote:
> > Hi,
> >
> > On 27/09/2019 18:47, Tomi Valkeinen wrote:
> >> On 27/09/2019 18:37, Tero Kristo wrote:
> >>
> >>> If you can provide details about what clock framework / driver does
> >>> wrong (sample clk_set_xyz call sequence, expected results via
> >>> clk_get_xyz, and what fails), I can take a look at it. Just reporting
> >>> arbitrary display driver issues I won't be able to debug at all (I
> >>> don't have access to any of the displays, nor do I want to waste time
> >>> debugging them without absolutely no knowledge whatsoever.)
> >>
> >> I used your hack patches to allow changing rates via debugfs. And set
> >> dss1_alwon_fck_3430es2 to 27000000 or 27870967. The end result was
> >> that DSS gets some very high clock from dss1_alwon_fck_3430es2, as the
> >> frame rate jumps to many hundreds fps.
> >>
> >> So, these numbers are not real, but to give the idea what I saw.
> >> Running first with 50 MHz, I can see, say, 40 fps. Then I set the
> >> clock to 30 MHz, and fps dropped to, say, 30fps, as expected with
> >> lower clock. Then I set the clock to 27MHz (or the other one),
> >> expecting a bit lower fps, but instead I saw hundreds of fps.
> >>
> >> I don't know if there's any other way to observe the wrong clock rate
> >> but have the dss enabled and running kmstest or similar. I can help
> >> you set that up next week, should be trivial. You don't need a display
> >> for that.
> >
> > Here's how to reproduce. I have the attached patches. Three of them are
> > the clk-debug ones, and one of mine to make it easy to test without a
> > display, and without underflow flood halting the device. There are on
> > top of v5.3. Kernel config also attached.
> >
> > kmstest is from kms++ project (https://github.com/tomba/kmsxx). It
> > should be straightforward to compile, but kmstest binary is also
> > included in TI's rootfs.
>
> Ok, I ignored all your test code and just fiddled with my trusty clk
> debugfs patches. I don't like debugging with test code I have no
> experience with. :)
>
> Anyways, it seems the dpll4_m4_ck max divider value is wrong, it only
> accepts values upto 16 at least on my board. The setting for this in DT
> is 32, and it is most likely SoC specific what happens if you write an
> invalid value to the divider.
>
> The best action here is probably to drop the max-div value for this
> clock to 16. Can someone check this with their display setup and see
> what happens? Attached patch should do the trick.

I tried your attached patch on my dm3730 and that seems to make it
somewhat better in that it doesn't hang anymore, so that leads me to
believe that your comment about the divider being only valid on the
omap36 may not be true. I do think it solves the hanging issue that i
was seeing, but I now see a new one now which is dumping a backtrace.

It looks like it's unhappy that its trying to get one frequency and
getting something different instead.

[   10.014099] WARNING: CPU: 0 PID: 111 at
drivers/gpu/drm/omapdrm/dss/dss.c:655 dss_set_fck_rate+0x70/0x90
[omapdss]
[   10.014129] clk rate mismatch: 27870968 != 27000000

See attached log for the full dump.

Either way, I think you've identified the main issue.  I just think we
may have uncovered another one in the process.

For what it's worth, the video looks good.  :-)

adam
>
> -Tero
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tero Kristo Sept. 30, 2019, 12:47 p.m. UTC | #20
On 30/09/2019 15:41, Adam Ford wrote:
> On Mon, Sep 30, 2019 at 3:53 AM Tero Kristo <t-kristo@ti.com> wrote:
>>
>> On 30/09/2019 09:45, Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On 27/09/2019 18:47, Tomi Valkeinen wrote:
>>>> On 27/09/2019 18:37, Tero Kristo wrote:
>>>>
>>>>> If you can provide details about what clock framework / driver does
>>>>> wrong (sample clk_set_xyz call sequence, expected results via
>>>>> clk_get_xyz, and what fails), I can take a look at it. Just reporting
>>>>> arbitrary display driver issues I won't be able to debug at all (I
>>>>> don't have access to any of the displays, nor do I want to waste time
>>>>> debugging them without absolutely no knowledge whatsoever.)
>>>>
>>>> I used your hack patches to allow changing rates via debugfs. And set
>>>> dss1_alwon_fck_3430es2 to 27000000 or 27870967. The end result was
>>>> that DSS gets some very high clock from dss1_alwon_fck_3430es2, as the
>>>> frame rate jumps to many hundreds fps.
>>>>
>>>> So, these numbers are not real, but to give the idea what I saw.
>>>> Running first with 50 MHz, I can see, say, 40 fps. Then I set the
>>>> clock to 30 MHz, and fps dropped to, say, 30fps, as expected with
>>>> lower clock. Then I set the clock to 27MHz (or the other one),
>>>> expecting a bit lower fps, but instead I saw hundreds of fps.
>>>>
>>>> I don't know if there's any other way to observe the wrong clock rate
>>>> but have the dss enabled and running kmstest or similar. I can help
>>>> you set that up next week, should be trivial. You don't need a display
>>>> for that.
>>>
>>> Here's how to reproduce. I have the attached patches. Three of them are
>>> the clk-debug ones, and one of mine to make it easy to test without a
>>> display, and without underflow flood halting the device. There are on
>>> top of v5.3. Kernel config also attached.
>>>
>>> kmstest is from kms++ project (https://github.com/tomba/kmsxx). It
>>> should be straightforward to compile, but kmstest binary is also
>>> included in TI's rootfs.
>>
>> Ok, I ignored all your test code and just fiddled with my trusty clk
>> debugfs patches. I don't like debugging with test code I have no
>> experience with. :)
>>
>> Anyways, it seems the dpll4_m4_ck max divider value is wrong, it only
>> accepts values upto 16 at least on my board. The setting for this in DT
>> is 32, and it is most likely SoC specific what happens if you write an
>> invalid value to the divider.
>>
>> The best action here is probably to drop the max-div value for this
>> clock to 16. Can someone check this with their display setup and see
>> what happens? Attached patch should do the trick.
> 
> I tried your attached patch on my dm3730 and that seems to make it
> somewhat better in that it doesn't hang anymore, so that leads me to
> believe that your comment about the divider being only valid on the
> omap36 may not be true. I do think it solves the hanging issue that i
> was seeing, but I now see a new one now which is dumping a backtrace.
> 
> It looks like it's unhappy that its trying to get one frequency and
> getting something different instead.
> 
> [   10.014099] WARNING: CPU: 0 PID: 111 at
> drivers/gpu/drm/omapdrm/dss/dss.c:655 dss_set_fck_rate+0x70/0x90
> [omapdss]
> [   10.014129] clk rate mismatch: 27870968 != 27000000

I believe this one is for Tomi to comment, his driver does some magic 
compares for the requested vs. actual received clock rates. If I am not 
mistaken, we are only modifying an integer divider here, and thus it is 
physically impossible to get accurate 27MHz rate to display.

-Tero

> 
> See attached log for the full dump.
> 
> Either way, I think you've identified the main issue.  I just think we
> may have uncovered another one in the process.
> 
> For what it's worth, the video looks good.  :-)
> 
> adam
>>
>> -Tero
>>
>> --

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Adam Ford Sept. 30, 2019, 1:17 p.m. UTC | #21
On Mon, Sep 30, 2019 at 7:48 AM Tero Kristo <t-kristo@ti.com> wrote:
>
> On 30/09/2019 15:41, Adam Ford wrote:
> > On Mon, Sep 30, 2019 at 3:53 AM Tero Kristo <t-kristo@ti.com> wrote:
> >>
> >> On 30/09/2019 09:45, Tomi Valkeinen wrote:
> >>> Hi,
> >>>
> >>> On 27/09/2019 18:47, Tomi Valkeinen wrote:
> >>>> On 27/09/2019 18:37, Tero Kristo wrote:
> >>>>
> >>>>> If you can provide details about what clock framework / driver does
> >>>>> wrong (sample clk_set_xyz call sequence, expected results via
> >>>>> clk_get_xyz, and what fails), I can take a look at it. Just reporting
> >>>>> arbitrary display driver issues I won't be able to debug at all (I
> >>>>> don't have access to any of the displays, nor do I want to waste time
> >>>>> debugging them without absolutely no knowledge whatsoever.)
> >>>>
> >>>> I used your hack patches to allow changing rates via debugfs. And set
> >>>> dss1_alwon_fck_3430es2 to 27000000 or 27870967. The end result was
> >>>> that DSS gets some very high clock from dss1_alwon_fck_3430es2, as the
> >>>> frame rate jumps to many hundreds fps.
> >>>>
> >>>> So, these numbers are not real, but to give the idea what I saw.
> >>>> Running first with 50 MHz, I can see, say, 40 fps. Then I set the
> >>>> clock to 30 MHz, and fps dropped to, say, 30fps, as expected with
> >>>> lower clock. Then I set the clock to 27MHz (or the other one),
> >>>> expecting a bit lower fps, but instead I saw hundreds of fps.
> >>>>
> >>>> I don't know if there's any other way to observe the wrong clock rate
> >>>> but have the dss enabled and running kmstest or similar. I can help
> >>>> you set that up next week, should be trivial. You don't need a display
> >>>> for that.
> >>>
> >>> Here's how to reproduce. I have the attached patches. Three of them are
> >>> the clk-debug ones, and one of mine to make it easy to test without a
> >>> display, and without underflow flood halting the device. There are on
> >>> top of v5.3. Kernel config also attached.
> >>>
> >>> kmstest is from kms++ project (https://github.com/tomba/kmsxx). It
> >>> should be straightforward to compile, but kmstest binary is also
> >>> included in TI's rootfs.
> >>
> >> Ok, I ignored all your test code and just fiddled with my trusty clk
> >> debugfs patches. I don't like debugging with test code I have no
> >> experience with. :)
> >>
> >> Anyways, it seems the dpll4_m4_ck max divider value is wrong, it only
> >> accepts values upto 16 at least on my board. The setting for this in DT
> >> is 32, and it is most likely SoC specific what happens if you write an
> >> invalid value to the divider.
> >>
> >> The best action here is probably to drop the max-div value for this
> >> clock to 16. Can someone check this with their display setup and see
> >> what happens? Attached patch should do the trick.
> >
> > I tried your attached patch on my dm3730 and that seems to make it
> > somewhat better in that it doesn't hang anymore, so that leads me to
> > believe that your comment about the divider being only valid on the
> > omap36 may not be true. I do think it solves the hanging issue that i
> > was seeing, but I now see a new one now which is dumping a backtrace.
> >
> > It looks like it's unhappy that its trying to get one frequency and
> > getting something different instead.
> >
> > [   10.014099] WARNING: CPU: 0 PID: 111 at
> > drivers/gpu/drm/omapdrm/dss/dss.c:655 dss_set_fck_rate+0x70/0x90
> > [omapdss]
> > [   10.014129] clk rate mismatch: 27870968 != 27000000
>
> I believe this one is for Tomi to comment, his driver does some magic
> compares for the requested vs. actual received clock rates. If I am not
> mistaken, we are only modifying an integer divider here, and thus it is
> physically impossible to get accurate 27MHz rate to display.

I didn't expect exactly 27MHz,but the back trace is what concerns me more.

However, looking at
# cat clk/dpll4_ck/clk_rate
864000000

It seems like 864000000 / 32 would be 27 MHz, but instead we're
dividing it by 31 yielding 27870968.  I don't know the clocking
architecture, so I don't know what your patch actually did or how the
divide by 16 limit worked into this.  If lck cannot divide by 32, it
would be nice to see if it could divide by 27 to get to 32MHz.  From
there, the pck could then divide by 4 yielding 9MHz.

adam

>
> -Tero
>
> >
> > See attached log for the full dump.
> >
> > Either way, I think you've identified the main issue.  I just think we
> > may have uncovered another one in the process.
> >
> > For what it's worth, the video looks good.  :-)
> >
> > adam
> >>
> >> -Tero
> >>
> >> --
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tomi Valkeinen Sept. 30, 2019, 1:35 p.m. UTC | #22
On 30/09/2019 16:17, Adam Ford wrote:

>>> It looks like it's unhappy that its trying to get one frequency and
>>> getting something different instead.
>>>
>>> [   10.014099] WARNING: CPU: 0 PID: 111 at
>>> drivers/gpu/drm/omapdrm/dss/dss.c:655 dss_set_fck_rate+0x70/0x90
>>> [omapdss]
>>> [   10.014129] clk rate mismatch: 27870968 != 27000000
>>
>> I believe this one is for Tomi to comment, his driver does some magic
>> compares for the requested vs. actual received clock rates. If I am not
>> mistaken, we are only modifying an integer divider here, and thus it is
>> physically impossible to get accurate 27MHz rate to display.
> 
> I didn't expect exactly 27MHz,but the back trace is what concerns me more.

Ah sorry... DSS driver knows the max divider value, so that it can 
iterate over all the rates to find a good one.

I'll send a patch later, but look for omap3630_dss_feats in dss.c, and 
change fck_div_max from 32 to 16.

> However, looking at
> # cat clk/dpll4_ck/clk_rate
> 864000000
> 
> It seems like 864000000 / 32 would be 27 MHz, but instead we're
> dividing it by 31 yielding 27870968.  I don't know the clocking
> architecture, so I don't know what your patch actually did or how the
> divide by 16 limit worked into this.  If lck cannot divide by 32, it
> would be nice to see if it could divide by 27 to get to 32MHz.  From
> there, the pck could then divide by 4 yielding 9MHz.

That's pretty odd. With Tero's patch (I didn't test it though) the max 
divider should be 16. So the minimum fclk rate should be 54MHz. But 
somehow the clock framework managed to produce 27870968...

  Tomi
H. Nikolaus Schaller Sept. 30, 2019, 1:38 p.m. UTC | #23
> Am 30.09.2019 um 10:53 schrieb Tero Kristo <t-kristo@ti.com>:
> 
> The best action here is probably to drop the max-div value for this clock to 16. Can someone check this with their display setup and see what happens? Attached patch should do the trick.

I have checked on GTA04 and OpenPandora (DM3730 resp. OMAP3430) and did not notice a negative effect.

(Well, we never see the problem that is discussed here and have built with CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK=0).

BR,
Nikolaus
Adam Ford Sept. 30, 2019, 2:04 p.m. UTC | #24
On Mon, Sep 30, 2019 at 8:54 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Sep 30, 2019 at 8:39 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> >
> >
> > > Am 30.09.2019 um 10:53 schrieb Tero Kristo <t-kristo@ti.com>:
> > >
> > > The best action here is probably to drop the max-div value for this clock to 16. Can someone check this with their display setup and see what happens? Attached patch should do the trick.
> >
> > I have checked on GTA04 and OpenPandora (DM3730 resp. OMAP3430) and did not notice a negative effect.
> >
> > (Well, we never see the problem that is discussed here and have built with CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK=0).
>
> I have never been able to use CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK=0, but
> I assume it's either a function of pck or a combination of pck with
> the resolution.
>
> Based on Tomi's comment, I assume he's working on the following.  Can
> you also try:
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c
> b/drivers/gpu/drm/omapdrm/dss/dss.c
> index 5711b7a720e6..5e584f32ea6a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> @@ -1090,7 +1090,7 @@ static const struct dss_features omap34xx_dss_feats = {
>
>  static const struct dss_features omap3630_dss_feats = {
>         .model                  =       DSS_MODEL_OMAP3,
> -       .fck_div_max            =       32,
> +       .fck_div_max            =       16,
>         .fck_freq_max           =       173000000,
>         .dss_fck_multiplier     =       1,
>         .parent_clk_name        =       "dpll4_ck",
>
>
> Hopefully it doesn't break the 3630 for you, but it fixed my issue
> with no back trace:
>
> [    9.915588] DSS: set fck to 54000000
> [    9.915618] DISPC: lck = 54000000 (1)
> [    9.915649] DISPC: pck = 9000000 (6)
> [    9.917633] DISPC: channel 0 xres 480 yres 272
> [    9.917663] DISPC: pck 9000000
>
> I do wonder, however if there is a divider that is higher than 16, but
> lower than 32.
> I was able to run fck at 36MHz before with divide by 4 to 9MHz, so I
> am hoping that by running at 54MHz / 6 doesn't draw more power.  I was
> reading through the datasheet, but I could not find any reference to
> the max divider.
>

For run, I tested a max divider of 27, and I was able to get it
functional with a slower fck

[    9.939056] DSS: set fck to 36000000
[    9.939086] DISPC: lck = 36000000 (1)
[    9.939086] DISPC: pck = 9000000 (4)
[    9.941314] DISPC: channel 0 xres 480 yres 272
[    9.941314] DISPC: pck 9000000
[    9.941314] DISPC: hsync_len 42 hfp 3 hbp 2 vsw 11 vfp 2 vbp 3
[    9.941314] DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1
de_level 1 sync_pclk_edge -1
[    9.941345] DISPC: hsync 17077Hz, vsync 59Hz


I don't know the implications, so if the people from TI say stick with
16, I'm fine with that, but at least there is some evidence that it
can be higher than 16, but lower than 32.

adam

> adam
> >
> > BR,
> > Nikolaus
> >
Adam Ford Sept. 30, 2019, 2:12 p.m. UTC | #25
On Mon, Sep 30, 2019 at 9:04 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Sep 30, 2019 at 8:54 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Mon, Sep 30, 2019 at 8:39 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
> > >
> > >
> > > > Am 30.09.2019 um 10:53 schrieb Tero Kristo <t-kristo@ti.com>:
> > > >
> > > > The best action here is probably to drop the max-div value for this clock to 16. Can someone check this with their display setup and see what happens? Attached patch should do the trick.
> > >
> > > I have checked on GTA04 and OpenPandora (DM3730 resp. OMAP3430) and did not notice a negative effect.
> > >
> > > (Well, we never see the problem that is discussed here and have built with CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK=0).
> >
> > I have never been able to use CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK=0, but
> > I assume it's either a function of pck or a combination of pck with
> > the resolution.
> >
> > Based on Tomi's comment, I assume he's working on the following.  Can
> > you also try:
> >
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dss.c
> > b/drivers/gpu/drm/omapdrm/dss/dss.c
> > index 5711b7a720e6..5e584f32ea6a 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dss.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/dss.c
> > @@ -1090,7 +1090,7 @@ static const struct dss_features omap34xx_dss_feats = {
> >
> >  static const struct dss_features omap3630_dss_feats = {
> >         .model                  =       DSS_MODEL_OMAP3,
> > -       .fck_div_max            =       32,
> > +       .fck_div_max            =       16,
> >         .fck_freq_max           =       173000000,
> >         .dss_fck_multiplier     =       1,
> >         .parent_clk_name        =       "dpll4_ck",
> >
> >
> > Hopefully it doesn't break the 3630 for you, but it fixed my issue
> > with no back trace:
> >
> > [    9.915588] DSS: set fck to 54000000
> > [    9.915618] DISPC: lck = 54000000 (1)
> > [    9.915649] DISPC: pck = 9000000 (6)
> > [    9.917633] DISPC: channel 0 xres 480 yres 272
> > [    9.917663] DISPC: pck 9000000
> >
> > I do wonder, however if there is a divider that is higher than 16, but
> > lower than 32.
> > I was able to run fck at 36MHz before with divide by 4 to 9MHz, so I
> > am hoping that by running at 54MHz / 6 doesn't draw more power.  I was
> > reading through the datasheet, but I could not find any reference to
> > the max divider.
> >
>
> For run, I tested a max divider of 27, and I was able to get it
> functional with a slower fck
>
> [    9.939056] DSS: set fck to 36000000
> [    9.939086] DISPC: lck = 36000000 (1)
> [    9.939086] DISPC: pck = 9000000 (4)
> [    9.941314] DISPC: channel 0 xres 480 yres 272
> [    9.941314] DISPC: pck 9000000
> [    9.941314] DISPC: hsync_len 42 hfp 3 hbp 2 vsw 11 vfp 2 vbp 3
> [    9.941314] DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1
> de_level 1 sync_pclk_edge -1
> [    9.941345] DISPC: hsync 17077Hz, vsync 59Hz
>
>
> I don't know the implications, so if the people from TI say stick with
> 16, I'm fine with that, but at least there is some evidence that it
> can be higher than 16, but lower than 32.
>

Sorry for all the spam, but I moved both of them to 31 from 32, and it
also seems to work successfully at 31.

[   26.923004] DSS: set fck to 36000000
[   26.923034] DISPC: lck = 36000000 (1)
[   26.923034] DISPC: pck = 9000000 (4)
[   26.925048] DISPC: channel 0 xres 480 yres 272
[   26.925048] DISPC: pck 9000000
[   26.925048] DISPC: hsync_len 42 hfp 3 hbp 2 vsw 11 vfp 2 vbp 3
[   26.925079] DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1
de_level 1 sync_pclk_edge -1
[   26.925079] DISPC: hsync 17077Hz, vsync 59Hz
[   27.384613] DISPC: dispc_runtime_put

Is it possible to use 31?

adam

> adam
>
> > adam
> > >
> > > BR,
> > > Nikolaus
> > >
Tomi Valkeinen Sept. 30, 2019, 2:20 p.m. UTC | #26
On 30/09/2019 17:12, Adam Ford wrote:

>> I don't know the implications, so if the people from TI say stick with
>> 16, I'm fine with that, but at least there is some evidence that it
>> can be higher than 16, but lower than 32.
>>
> 
> Sorry for all the spam, but I moved both of them to 31 from 32, and it
> also seems to work successfully at 31.
> 
> [   26.923004] DSS: set fck to 36000000
> [   26.923034] DISPC: lck = 36000000 (1)
> [   26.923034] DISPC: pck = 9000000 (4)
> [   26.925048] DISPC: channel 0 xres 480 yres 272
> [   26.925048] DISPC: pck 9000000
> [   26.925048] DISPC: hsync_len 42 hfp 3 hbp 2 vsw 11 vfp 2 vbp 3
> [   26.925079] DISPC: vsync_level 1 hsync_level 1 data_pclk_edge 1
> de_level 1 sync_pclk_edge -1
> [   26.925079] DISPC: hsync 17077Hz, vsync 59Hz
> [   27.384613] DISPC: dispc_runtime_put
> 
> Is it possible to use 31?

Let's see what Tero says, but yeah, something is odd here. I expected 
the max divider to be 16 with Tero's patch, but I don't see it having 
that effect. I can get the div to 31.

You can see this from the clock register 0x48004e40 (CM_CLKSEL_DSS). The 
lowest bits are the divider, 5 to 0. The TRM says max div is 32.

Tero said for him the dividers > 16 didn't "stick" to the register. I'm 
now wondering if he has an old beagleboard with OMAP34xx, which has max 
div 16.

  Tomi
Tomi Valkeinen Sept. 30, 2019, 2:27 p.m. UTC | #27
On 30/09/2019 17:20, Tomi Valkeinen wrote:

> Let's see what Tero says, but yeah, something is odd here. I expected 
> the max divider to be 16 with Tero's patch, but I don't see it having 
> that effect. I can get the div to 31.
> 
> You can see this from the clock register 0x48004e40 (CM_CLKSEL_DSS). The 
> lowest bits are the divider, 5 to 0. The TRM says max div is 32.
> 
> Tero said for him the dividers > 16 didn't "stick" to the register. I'm 
> now wondering if he has an old beagleboard with OMAP34xx, which has max 
> div 16.

So testing a bit more here, I can see the DSS working fine and fps as 
expected when I write values directly to CM_CLKSEL_DSS:5:0, with 
dividers up to 31. With 32, DSS breaks. The TRM (AM/DM37x) says value 32 
is valid.

  Tomi
H. Nikolaus Schaller Sept. 30, 2019, 2:56 p.m. UTC | #28
> Am 30.09.2019 um 16:27 schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>:
> 
> On 30/09/2019 17:20, Tomi Valkeinen wrote:
> 
>> Let's see what Tero says, but yeah, something is odd here. I expected the max divider to be 16 with Tero's patch, but I don't see it having that effect. I can get the div to 31.
>> You can see this from the clock register 0x48004e40 (CM_CLKSEL_DSS). The lowest bits are the divider, 5 to 0. The TRM says max div is 32.
>> Tero said for him the dividers > 16 didn't "stick" to the register. I'm now wondering if he has an old beagleboard with OMAP34xx, which has max div 16.
> 
> So testing a bit more here, I can see the DSS working fine and fps as expected when I write values directly to CM_CLKSEL_DSS:5:0, with dividers up to 31. With 32, DSS breaks. The TRM (AM/DM37x) says value 32 is valid.

Just a blind guess: is there something in the errata to take care of?
Adam Ford Sept. 30, 2019, 3:10 p.m. UTC | #29
On Mon, Sep 30, 2019 at 9:27 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> On 30/09/2019 17:20, Tomi Valkeinen wrote:
>
> > Let's see what Tero says, but yeah, something is odd here. I expected
> > the max divider to be 16 with Tero's patch, but I don't see it having
> > that effect. I can get the div to 31.
> >
> > You can see this from the clock register 0x48004e40 (CM_CLKSEL_DSS). The
> > lowest bits are the divider, 5 to 0. The TRM says max div is 32.
> >
> > Tero said for him the dividers > 16 didn't "stick" to the register. I'm
> > now wondering if he has an old beagleboard with OMAP34xx, which has max
> > div 16.
>
> So testing a bit more here, I can see the DSS working fine and fps as
> expected when I write values directly to CM_CLKSEL_DSS:5:0, with
> dividers up to 31. With 32, DSS breaks. The TRM (AM/DM37x) says value 32
> is valid.

I wonder if it's somehow being masked with bits 4:0 instead of 5:0
which could potentially make the divider 0 and that value doesn't
appear to be valid.

adam

>
>   Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tero Kristo Sept. 30, 2019, 5:48 p.m. UTC | #30
On 30/09/2019 18:10, Adam Ford wrote:
> On Mon, Sep 30, 2019 at 9:27 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>
>> On 30/09/2019 17:20, Tomi Valkeinen wrote:
>>
>>> Let's see what Tero says, but yeah, something is odd here. I expected
>>> the max divider to be 16 with Tero's patch, but I don't see it having
>>> that effect. I can get the div to 31.
>>>
>>> You can see this from the clock register 0x48004e40 (CM_CLKSEL_DSS). The
>>> lowest bits are the divider, 5 to 0. The TRM says max div is 32.
>>>
>>> Tero said for him the dividers > 16 didn't "stick" to the register. I'm
>>> now wondering if he has an old beagleboard with OMAP34xx, which has max
>>> div 16.
>>
>> So testing a bit more here, I can see the DSS working fine and fps as
>> expected when I write values directly to CM_CLKSEL_DSS:5:0, with
>> dividers up to 31. With 32, DSS breaks. The TRM (AM/DM37x) says value 32
>> is valid.
> 
> I wonder if it's somehow being masked with bits 4:0 instead of 5:0
> which could potentially make the divider 0 and that value doesn't
> appear to be valid.

Hmmh, after some testing, it seems there is bad stuff happening with the 
divider clock implementation, I am re-working it as of now. Basically 
what is wrong is that with a divider max value of say 16, the driver 
attempts to craft the max value into a mask, but this ends up being 
0x1f. If the max value is 15, it ends up into 0xf which is correct.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tomi Valkeinen Oct. 1, 2019, 5:07 a.m. UTC | #31
On 30/09/2019 20:48, Tero Kristo wrote:

> Hmmh, after some testing, it seems there is bad stuff happening with the 
> divider clock implementation, I am re-working it as of now. Basically 
> what is wrong is that with a divider max value of say 16, the driver 
> attempts to craft the max value into a mask, but this ends up being 
> 0x1f. If the max value is 15, it ends up into 0xf which is correct.

Ok, that explains the max not working.

It doesn't explain the other issue, where the TRM says the max div is 
32, but it does not work. But taking the max div from the old SoCs, 16, 
is not correct either, as it seems that dividers up to 31 work ok.

  Tomi
Tero Kristo Oct. 1, 2019, 5:12 a.m. UTC | #32
On 01/10/2019 08:07, Tomi Valkeinen wrote:
> On 30/09/2019 20:48, Tero Kristo wrote:
> 
>> Hmmh, after some testing, it seems there is bad stuff happening with 
>> the divider clock implementation, I am re-working it as of now. 
>> Basically what is wrong is that with a divider max value of say 16, 
>> the driver attempts to craft the max value into a mask, but this ends 
>> up being 0x1f. If the max value is 15, it ends up into 0xf which is 
>> correct.
> 
> Ok, that explains the max not working.
> 
> It doesn't explain the other issue, where the TRM says the max div is 
> 32, but it does not work. But taking the max div from the old SoCs, 16, 
> is not correct either, as it seems that dividers up to 31 work ok.

If someone knows for sure that dividers higher than 16 are fine on 
omap36xx, we can add this under omap36xx-clocks.dtsi. Anyway, let me fix 
the broken divider max logic first, that seems to be more pressing issue.

-Tero

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tomi Valkeinen Oct. 1, 2019, 9:31 a.m. UTC | #33
On 01/10/2019 11:12, Tero Kristo wrote:
> On 01/10/2019 08:07, Tomi Valkeinen wrote:
>> On 30/09/2019 20:48, Tero Kristo wrote:
>>
>>> Hmmh, after some testing, it seems there is bad stuff happening with 
>>> the divider clock implementation, I am re-working it as of now. 
>>> Basically what is wrong is that with a divider max value of say 16, 
>>> the driver attempts to craft the max value into a mask, but this ends 
>>> up being 0x1f. If the max value is 15, it ends up into 0xf which is 
>>> correct.
>>
>> Ok, that explains the max not working.
>>
>> It doesn't explain the other issue, where the TRM says the max div is 
>> 32, but it does not work. But taking the max div from the old SoCs, 
>> 16, is not correct either, as it seems that dividers up to 31 work ok.
>>
>>   Tomi
>>
> 
> Ok, attached a series that hopefully fixes it, any testing feedback 
> welcome before I post this properly.
> 
> This also supports omap36xx dpll4_m4_ck divider up-to 31, other omap3 
> family is limited to 16.

Works for me. This also needs the change to dss.c to change the max from 
32 to 31. I'll send a patch for that separately.

  Tomi
Adam Ford Oct. 1, 2019, 1:06 p.m. UTC | #34
On Tue, Oct 1, 2019 at 4:31 AM Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> On 01/10/2019 11:12, Tero Kristo wrote:
> > On 01/10/2019 08:07, Tomi Valkeinen wrote:
> >> On 30/09/2019 20:48, Tero Kristo wrote:
> >>
> >>> Hmmh, after some testing, it seems there is bad stuff happening with
> >>> the divider clock implementation, I am re-working it as of now.
> >>> Basically what is wrong is that with a divider max value of say 16,
> >>> the driver attempts to craft the max value into a mask, but this ends
> >>> up being 0x1f. If the max value is 15, it ends up into 0xf which is
> >>> correct.
> >>
> >> Ok, that explains the max not working.
> >>
> >> It doesn't explain the other issue, where the TRM says the max div is
> >> 32, but it does not work. But taking the max div from the old SoCs,
> >> 16, is not correct either, as it seems that dividers up to 31 work ok.
> >>
> >>   Tomi
> >>
> >
> > Ok, attached a series that hopefully fixes it, any testing feedback
> > welcome before I post this properly.
> >
> > This also supports omap36xx dpll4_m4_ck divider up-to 31, other omap3
> > family is limited to 16.

Thank you!  This works for me.

>
> Works for me. This also needs the change to dss.c to change the max from
> 32 to 31. I'll send a patch for that separately.

Tomi,

Do you want me to push a patch to remove the
CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK hack once these patches have been
posted?  It seems like the divider fix eliminates the need for this
hack.

adam
>
>   Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Tomi Valkeinen Oct. 1, 2019, 1:14 p.m. UTC | #35
On 01/10/2019 16:06, Adam Ford wrote:

> Do you want me to push a patch to remove the
> CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK hack once these patches have been
> posted?  It seems like the divider fix eliminates the need for this
> hack.

No, the point of OMAP2_DSS_MIN_FCK_PER_PCK was never to work around 
divider bugs. It's for scaling.

  Tomi
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
index cd02516a40b6..42449d07c47e 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
@@ -40,7 +40,7 @@  Required properties:
 Optional properties:
 - max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth limit
 			in bytes per second
-
+- min-fck-pck-ratio:  Make sure that DISPC FCK is at least n x PCK
 
 RFBI
 ----
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 4043ecb38016..bf84a8487aae 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -751,7 +751,7 @@ 
 			#size-cells = <1>;
 			ranges;
 
-			dispc@48050400 {
+			dispc: dispc@48050400 {
 				compatible = "ti,omap3-dispc";
 				reg = <0x48050400 0x400>;
 				interrupts = <25>;
diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
index f24ebf7f61dd..d0666edcdf2a 100644
--- a/drivers/gpu/drm/omapdrm/dss/Kconfig
+++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
@@ -102,24 +102,6 @@  config OMAP2_DSS_DSI
 
 	  See http://www.mipi.org/ for DSI specifications.
 
-config OMAP2_DSS_MIN_FCK_PER_PCK
-	int "Minimum FCK/PCK ratio (for scaling)"
-	range 0 32
-	default 0
-	help
-	  This can be used to adjust the minimum FCK/PCK ratio.
-
-	  With this you can make sure that DISPC FCK is at least
-	  n x PCK. Video plane scaling requires higher FCK than
-	  normally.
-
-	  If this is set to 0, there's no extra constraint on the
-	  DISPC FCK. However, the FCK will at minimum be
-	  2xPCK (if active matrix) or 3xPCK (if passive matrix).
-
-	  Max FCK is 173MHz, so this doesn't work if your PCK
-	  is very high.
-
 config OMAP2_DSS_SLEEP_AFTER_VENC_RESET
 	bool "Sleep 20ms after VENC reset"
 	default y
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index ba82d916719c..09a130c53da2 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -198,6 +198,9 @@  struct dispc_device {
 
 	/* DISPC_CONTROL & DISPC_CONFIG lock*/
 	spinlock_t control_lock;
+
+	/* Optional min-fck-pck-ratio */
+	u32 min_fck_per_pck;
 };
 
 enum omap_color_component {
@@ -3683,15 +3686,8 @@  bool dispc_div_calc(struct dispc_device *dispc, unsigned long dispc_freq,
 	unsigned long pck, lck;
 	unsigned long lck_max;
 	unsigned long pckd_hw_min, pckd_hw_max;
-	unsigned int min_fck_per_pck;
 	unsigned long fck;
 
-#ifdef CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK
-	min_fck_per_pck = CONFIG_OMAP2_DSS_MIN_FCK_PER_PCK;
-#else
-	min_fck_per_pck = 0;
-#endif
-
 	pckd_hw_min = dispc->feat->min_pcd;
 	pckd_hw_max = 255;
 
@@ -3723,7 +3719,7 @@  bool dispc_div_calc(struct dispc_device *dispc, unsigned long dispc_freq,
 			else
 				fck = lck;
 
-			if (fck < pck * min_fck_per_pck)
+			if (fck < pck * dispc->min_fck_per_pck)
 				continue;
 
 			if (func(lckd, pckd, lck, pck, data))
@@ -4826,6 +4822,8 @@  static int dispc_bind(struct device *dev, struct device *master, void *data)
 		}
 	}
 
+	of_property_read_u32(np, "min-fck-pck-ratio", &dispc->min_fck_per_pck);
+
 	r = dispc_init_gamma_tables(dispc);
 	if (r)
 		goto err_free;