diff mbox

[RFC,07/11] drm: sun4i: add support for the TV encoder in H3 SoC

Message ID 20170517164354.16399-8-icenowy@aosc.io (mailing list archive)
State New, archived
Headers show

Commit Message

Icenowy Zheng May 17, 2017, 4:43 p.m. UTC
Allwinner H3 features a TV encoder similar to the one in earlier SoCs,
but with some different points about clocks:
- It has a mod clock and a bus clock.
- The mod clock must be at a fixed rate to generate signal.

Add support for it.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
 drivers/gpu/drm/sun4i/sun4i_tv.c | 65 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

Comments

Maxime Ripard May 19, 2017, 6:03 p.m. UTC | #1
On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> Allwinner H3 features a TV encoder similar to the one in earlier SoCs,
> but with some different points about clocks:
> - It has a mod clock and a bus clock.
> - The mod clock must be at a fixed rate to generate signal.

Why?

Maxime
Icenowy Zheng May 19, 2017, 6:08 p.m. UTC | #2
于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> Allwinner H3 features a TV encoder similar to the one in earlier
>SoCs,
>> but with some different points about clocks:
>> - It has a mod clock and a bus clock.
>> - The mod clock must be at a fixed rate to generate signal.
>
>Why?

It's experiment result by Jernej.

The clock rates in BSP kernel is also specially designed
(PLL_DE at 432MHz) in order to be able to feed the TVE.

>
>Maxime
Jernej Škrabec May 19, 2017, 6:23 p.m. UTC | #3
Hi,

Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
electrons.com> 写到:
> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> >> Allwinner H3 features a TV encoder similar to the one in earlier
> >
> >SoCs,
> >
> >> but with some different points about clocks:
> >> - It has a mod clock and a bus clock.
> >> - The mod clock must be at a fixed rate to generate signal.
> >
> >Why?
> 
> It's experiment result by Jernej.
> 
> The clock rates in BSP kernel is also specially designed
> (PLL_DE at 432MHz) in order to be able to feed the TVE.

My experiments and search through BSP code showed that TVE seems to have 
additional fixed predivider 8. So if you want to generate 27 MHz clock, unit 
has to be feed with 216 MHz. 

TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for DE2, 
BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz. This 
clock is then divided by 8 internaly to get final 27 MHz.

Please note that I don't have any hard evidence to support that, only 
experimental data. However, only that explanation make sense to me.

BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz base 
clock. Further experiments are needed to check if there is any possibility to 
have other resolutions by manipulating clocks and give other proper settings. 
I plan to do that, but not in very near future.

Best regards,
Jernej
Chen-Yu Tsai May 20, 2017, 1:37 a.m. UTC | #4
On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec <jernej.skrabec@siol.net> wrote:
> Hi,
>
> Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
>> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
> electrons.com> 写到:
>> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> >> Allwinner H3 features a TV encoder similar to the one in earlier
>> >
>> >SoCs,
>> >
>> >> but with some different points about clocks:
>> >> - It has a mod clock and a bus clock.
>> >> - The mod clock must be at a fixed rate to generate signal.
>> >
>> >Why?
>>
>> It's experiment result by Jernej.
>>
>> The clock rates in BSP kernel is also specially designed
>> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>
> My experiments and search through BSP code showed that TVE seems to have
> additional fixed predivider 8. So if you want to generate 27 MHz clock, unit
> has to be feed with 216 MHz.
>
> TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for DE2,
> BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz. This
> clock is then divided by 8 internaly to get final 27 MHz.
>
> Please note that I don't have any hard evidence to support that, only
> experimental data. However, only that explanation make sense to me.
>
> BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz base
> clock. Further experiments are needed to check if there is any possibility to
> have other resolutions by manipulating clocks and give other proper settings.
> I plan to do that, but not in very near future.

You only have composite video output, and those are the only 2 standard
resolutions that make any sense.

ChenYu
Jernej Škrabec May 22, 2017, 5:55 p.m. UTC | #5
Hi,

Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec <jernej.skrabec@siol.net> 
wrote:
> > Hi,
> > 
> > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
> > 
> > electrons.com> 写到:
> >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> >> >
> >> >SoCs,
> >> >
> >> >> but with some different points about clocks:
> >> >> - It has a mod clock and a bus clock.
> >> >> - The mod clock must be at a fixed rate to generate signal.
> >> >
> >> >Why?
> >> 
> >> It's experiment result by Jernej.
> >> 
> >> The clock rates in BSP kernel is also specially designed
> >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > 
> > My experiments and search through BSP code showed that TVE seems to have
> > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > unit has to be feed with 216 MHz.
> > 
> > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > DE2,
> > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > This clock is then divided by 8 internaly to get final 27 MHz.
> > 
> > Please note that I don't have any hard evidence to support that, only
> > experimental data. However, only that explanation make sense to me.
> > 
> > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > base clock. Further experiments are needed to check if there is any
> > possibility to have other resolutions by manipulating clocks and give
> > other proper settings. I plan to do that, but not in very near future.
> 
> You only have composite video output, and those are the only 2 standard
> resolutions that make any sense.

Right, other resolutions are for VGA.

Anyway, I did some more digging in A10 and R40 datasheets. I think that H3 TVE 
unit is something in between. R40 TVE has a setting to select "up sample". 
Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver on R40 
has this setting enabled only for PAL and NTSC and it is always 216 MHz. I 
think that H3 may have this hardwired to 216 MHz and this would be the reason 
why 216 MHz is needed.

Has anyone else any better explanation?

Best regards,
Jernej
Maxime Ripard May 23, 2017, 12:53 p.m. UTC | #6
On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
> Hi,
> 
> Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec <jernej.skrabec@siol.net> 
> wrote:
> > > Hi,
> > > 
> > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
> > > 
> > > electrons.com> 写到:
> > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> > >> >
> > >> >SoCs,
> > >> >
> > >> >> but with some different points about clocks:
> > >> >> - It has a mod clock and a bus clock.
> > >> >> - The mod clock must be at a fixed rate to generate signal.
> > >> >
> > >> >Why?
> > >> 
> > >> It's experiment result by Jernej.
> > >> 
> > >> The clock rates in BSP kernel is also specially designed
> > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > 
> > > My experiments and search through BSP code showed that TVE seems to have
> > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > > unit has to be feed with 216 MHz.
> > > 
> > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > > DE2,
> > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > > This clock is then divided by 8 internaly to get final 27 MHz.
> > > 
> > > Please note that I don't have any hard evidence to support that, only
> > > experimental data. However, only that explanation make sense to me.
> > > 
> > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > > base clock. Further experiments are needed to check if there is any
> > > possibility to have other resolutions by manipulating clocks and give
> > > other proper settings. I plan to do that, but not in very near future.
> > 
> > You only have composite video output, and those are the only 2 standard
> > resolutions that make any sense.
> 
> Right, other resolutions are for VGA.
> 
> Anyway, I did some more digging in A10 and R40 datasheets. I think that H3 TVE 
> unit is something in between. R40 TVE has a setting to select "up sample".

That might be just another translation of oversampling :)

I didn't know it could be applied to composite signals though, but I
guess this is just another analog signal after all.

> Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver on R40 
> has this setting enabled only for PAL and NTSC and it is always 216 MHz. I 
> think that H3 may have this hardwired to 216 MHz and this would be the reason 
> why 216 MHz is needed.
> 
> Has anyone else any better explanation?

That's already a pretty good one.

Either way, wether this is upsampling, oversampling or just a
pre-divider, this can and should be dealt with in the mode_set
callback, and not in the probe.

Thanks!
Maxime
Icenowy Zheng May 23, 2017, 12:56 p.m. UTC | #7
于 2017年5月23日 GMT+08:00 下午8:53:21, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
>> Hi,
>> 
>> Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
>> > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec
><jernej.skrabec@siol.net> 
>> wrote:
>> > > Hi,
>> > > 
>> > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
>napisal(a):
>> > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard
><maxime.ripard@free-
>> > > 
>> > > electrons.com> 写到:
>> > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> > >> >> Allwinner H3 features a TV encoder similar to the one in
>earlier
>> > >> >
>> > >> >SoCs,
>> > >> >
>> > >> >> but with some different points about clocks:
>> > >> >> - It has a mod clock and a bus clock.
>> > >> >> - The mod clock must be at a fixed rate to generate signal.
>> > >> >
>> > >> >Why?
>> > >> 
>> > >> It's experiment result by Jernej.
>> > >> 
>> > >> The clock rates in BSP kernel is also specially designed
>> > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > > 
>> > > My experiments and search through BSP code showed that TVE seems
>to have
>> > > additional fixed predivider 8. So if you want to generate 27 MHz
>clock,
>> > > unit has to be feed with 216 MHz.
>> > > 
>> > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit
>low for
>> > > DE2,
>> > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate
>216 MHz.
>> > > This clock is then divided by 8 internaly to get final 27 MHz.
>> > > 
>> > > Please note that I don't have any hard evidence to support that,
>only
>> > > experimental data. However, only that explanation make sense to
>me.
>> > > 
>> > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both
>use 27 MHz
>> > > base clock. Further experiments are needed to check if there is
>any
>> > > possibility to have other resolutions by manipulating clocks and
>give
>> > > other proper settings. I plan to do that, but not in very near
>future.
>> > 
>> > You only have composite video output, and those are the only 2
>standard
>> > resolutions that make any sense.
>> 
>> Right, other resolutions are for VGA.
>> 
>> Anyway, I did some more digging in A10 and R40 datasheets. I think
>that H3 TVE 
>> unit is something in between. R40 TVE has a setting to select "up
>sample".
>
>That might be just another translation of oversampling :)
>
>I didn't know it could be applied to composite signals though, but I
>guess this is just another analog signal after all.
>
>> Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver
>on R40 
>> has this setting enabled only for PAL and NTSC and it is always 216
>MHz. I 
>> think that H3 may have this hardwired to 216 MHz and this would be
>the reason 
>> why 216 MHz is needed.
>> 
>> Has anyone else any better explanation?
>
>That's already a pretty good one.
>
>Either way, wether this is upsampling, oversampling or just a
>pre-divider, this can and should be dealt with in the mode_set
>callback, and not in the probe.

What should we do for this?

Add a hook in TCON driver and let TVE driver affect the clock value (*16, as the dotclock is halfed)?

>
>Thanks!
>Maxime
Icenowy Zheng May 23, 2017, 1 p.m. UTC | #8
在 2017-05-23 20:53,Maxime Ripard 写道:
> On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
>> Hi,
>> 
>> Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
>> > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec <jernej.skrabec@siol.net>
>> wrote:
>> > > Hi,
>> > >
>> > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
>> > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
>> > >
>> > > electrons.com> 写到:
>> > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
>> > >> >
>> > >> >SoCs,
>> > >> >
>> > >> >> but with some different points about clocks:
>> > >> >> - It has a mod clock and a bus clock.
>> > >> >> - The mod clock must be at a fixed rate to generate signal.
>> > >> >
>> > >> >Why?
>> > >>
>> > >> It's experiment result by Jernej.
>> > >>
>> > >> The clock rates in BSP kernel is also specially designed
>> > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > >
>> > > My experiments and search through BSP code showed that TVE seems to have
>> > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
>> > > unit has to be feed with 216 MHz.
>> > >
>> > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
>> > > DE2,
>> > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
>> > > This clock is then divided by 8 internaly to get final 27 MHz.
>> > >
>> > > Please note that I don't have any hard evidence to support that, only
>> > > experimental data. However, only that explanation make sense to me.
>> > >
>> > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
>> > > base clock. Further experiments are needed to check if there is any
>> > > possibility to have other resolutions by manipulating clocks and give
>> > > other proper settings. I plan to do that, but not in very near future.
>> >
>> > You only have composite video output, and those are the only 2 standard
>> > resolutions that make any sense.
>> 
>> Right, other resolutions are for VGA.
>> 
>> Anyway, I did some more digging in A10 and R40 datasheets. I think 
>> that H3 TVE
>> unit is something in between. R40 TVE has a setting to select "up 
>> sample".
> 
> That might be just another translation of oversampling :)
> 
> I didn't know it could be applied to composite signals though, but I
> guess this is just another analog signal after all.
> 
>> Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP driver 
>> on R40
>> has this setting enabled only for PAL and NTSC and it is always 216 
>> MHz. I
>> think that H3 may have this hardwired to 216 MHz and this would be the 
>> reason
>> why 216 MHz is needed.
>> 
>> Has anyone else any better explanation?
> 
> That's already a pretty good one.
> 
> Either way, wether this is upsampling, oversampling or just a
> pre-divider, this can and should be dealt with in the mode_set
> callback, and not in the probe.

I got a better idea -- let TVE driver have the CLK_TVE as an
input and create a subclock output with divider 16, and feed this
subclock to TCON lcd-ch1.

This is a model of the real hardware -- the clock divider is in
TVE, not TCON.

> 
> Thanks!
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Maxime Ripard May 24, 2017, 7:30 a.m. UTC | #9
On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
> 在 2017-05-23 20:53,Maxime Ripard 写道:
> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
> > > Hi,
> > > 
> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec <jernej.skrabec@siol.net>
> > > wrote:
> > > > > Hi,
> > > > >
> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
> > > > >
> > > > > electrons.com> 写到:
> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> > > > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> > > > >> >
> > > > >> >SoCs,
> > > > >> >
> > > > >> >> but with some different points about clocks:
> > > > >> >> - It has a mod clock and a bus clock.
> > > > >> >> - The mod clock must be at a fixed rate to generate signal.
> > > > >> >
> > > > >> >Why?
> > > > >>
> > > > >> It's experiment result by Jernej.
> > > > >>
> > > > >> The clock rates in BSP kernel is also specially designed
> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > > >
> > > > > My experiments and search through BSP code showed that TVE seems to have
> > > > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > > > > unit has to be feed with 216 MHz.
> > > > >
> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > > > > DE2,
> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > > > > This clock is then divided by 8 internaly to get final 27 MHz.
> > > > >
> > > > > Please note that I don't have any hard evidence to support that, only
> > > > > experimental data. However, only that explanation make sense to me.
> > > > >
> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > > > > base clock. Further experiments are needed to check if there is any
> > > > > possibility to have other resolutions by manipulating clocks and give
> > > > > other proper settings. I plan to do that, but not in very near future.
> > > >
> > > > You only have composite video output, and those are the only 2 standard
> > > > resolutions that make any sense.
> > > 
> > > Right, other resolutions are for VGA.
> > > 
> > > Anyway, I did some more digging in A10 and R40 datasheets. I think
> > > that H3 TVE
> > > unit is something in between. R40 TVE has a setting to select "up
> > > sample".
> > 
> > That might be just another translation of oversampling :)
> > 
> > I didn't know it could be applied to composite signals though, but I
> > guess this is just another analog signal after all.
> > 
> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> > > driver on R40
> > > has this setting enabled only for PAL and NTSC and it is always 216
> > > MHz. I
> > > think that H3 may have this hardwired to 216 MHz and this would be
> > > the reason
> > > why 216 MHz is needed.
> > > 
> > > Has anyone else any better explanation?
> > 
> > That's already a pretty good one.
> > 
> > Either way, wether this is upsampling, oversampling or just a
> > pre-divider, this can and should be dealt with in the mode_set
> > callback, and not in the probe.
> 
> I got a better idea -- let TVE driver have the CLK_TVE as an
> input and create a subclock output with divider 16, and feed this
> subclock to TCON lcd-ch1.
> 
> This is a model of the real hardware -- the clock divider is in
> TVE, not TCON.

That's definitely not a good representation of the hardware. There's
one clock, it goes to the TCON, period.

However, the TV encoder has a constraint on that clock rate. This can
be easily implemented using a custom encoder state where you'd set the
multiplier to set on that clock, and the TCON will use it.

Maxime
Icenowy Zheng May 24, 2017, 8:25 a.m. UTC | #10
于 2017年5月24日 GMT+08:00 下午3:30:19, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
>On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
>> 在 2017-05-23 20:53,Maxime Ripard 写道:
>> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
>> > > Hi,
>> > > 
>> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
>napisal(a):
>> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec
><jernej.skrabec@siol.net>
>> > > wrote:
>> > > > > Hi,
>> > > > >
>> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
>napisal(a):
>> > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard
><maxime.ripard@free-
>> > > > >
>> > > > > electrons.com> 写到:
>> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
>wrote:
>> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
>earlier
>> > > > >> >
>> > > > >> >SoCs,
>> > > > >> >
>> > > > >> >> but with some different points about clocks:
>> > > > >> >> - It has a mod clock and a bus clock.
>> > > > >> >> - The mod clock must be at a fixed rate to generate
>signal.
>> > > > >> >
>> > > > >> >Why?
>> > > > >>
>> > > > >> It's experiment result by Jernej.
>> > > > >>
>> > > > >> The clock rates in BSP kernel is also specially designed
>> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > > > >
>> > > > > My experiments and search through BSP code showed that TVE
>seems to have
>> > > > > additional fixed predivider 8. So if you want to generate 27
>MHz clock,
>> > > > > unit has to be feed with 216 MHz.
>> > > > >
>> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
>bit low for
>> > > > > DE2,
>> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
>generate 216 MHz.
>> > > > > This clock is then divided by 8 internaly to get final 27
>MHz.
>> > > > >
>> > > > > Please note that I don't have any hard evidence to support
>that, only
>> > > > > experimental data. However, only that explanation make sense
>to me.
>> > > > >
>> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
>both use 27 MHz
>> > > > > base clock. Further experiments are needed to check if there
>is any
>> > > > > possibility to have other resolutions by manipulating clocks
>and give
>> > > > > other proper settings. I plan to do that, but not in very
>near future.
>> > > >
>> > > > You only have composite video output, and those are the only 2
>standard
>> > > > resolutions that make any sense.
>> > > 
>> > > Right, other resolutions are for VGA.
>> > > 
>> > > Anyway, I did some more digging in A10 and R40 datasheets. I
>think
>> > > that H3 TVE
>> > > unit is something in between. R40 TVE has a setting to select "up
>> > > sample".
>> > 
>> > That might be just another translation of oversampling :)
>> > 
>> > I didn't know it could be applied to composite signals though, but
>I
>> > guess this is just another analog signal after all.
>> > 
>> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
>> > > driver on R40
>> > > has this setting enabled only for PAL and NTSC and it is always
>216
>> > > MHz. I
>> > > think that H3 may have this hardwired to 216 MHz and this would
>be
>> > > the reason
>> > > why 216 MHz is needed.
>> > > 
>> > > Has anyone else any better explanation?
>> > 
>> > That's already a pretty good one.
>> > 
>> > Either way, wether this is upsampling, oversampling or just a
>> > pre-divider, this can and should be dealt with in the mode_set
>> > callback, and not in the probe.
>> 
>> I got a better idea -- let TVE driver have the CLK_TVE as an
>> input and create a subclock output with divider 16, and feed this
>> subclock to TCON lcd-ch1.
>> 
>> This is a model of the real hardware -- the clock divider is in
>> TVE, not TCON.
>
>That's definitely not a good representation of the hardware. There's
>one clock, it goes to the TCON, period.

No, I still think it goes to the TVE as:

1. it's named TVE in datasheet.
2. Generating signal with such a low resolution but such
a high dotclock is not a good situation.

>
>However, the TV encoder has a constraint on that clock rate. This can
>be easily implemented using a custom encoder state where you'd set the
>multiplier to set on that clock, and the TCON will use it.
>
>Maxime
Jernej Škrabec May 24, 2017, 3:23 p.m. UTC | #11
Hi,

Dne sreda, 24. maj 2017 ob 10:25:46 CEST je Icenowy Zheng napisal(a):
> 于 2017年5月24日 GMT+08:00 下午3:30:19, Maxime Ripard <maxime.ripard@free-
electrons.com> 写到:
> >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
> >> 在 2017-05-23 20:53,Maxime Ripard 写道:
> >> 
> >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
> >> > > Hi,
> >> > > 
> >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
> >
> >napisal(a):
> >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec
> >
> ><jernej.skrabec@siol.net>
> >
> >> > > wrote:
> >> > > > > Hi,
> >> > > > > 
> >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
> >
> >napisal(a):
> >> > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard
> >
> ><maxime.ripard@free-
> >
> >> > > > > electrons.com> 写到:
> >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
> >
> >wrote:
> >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
> >
> >earlier
> >
> >> > > > >> >SoCs,
> >> > > > >> >
> >> > > > >> >> but with some different points about clocks:
> >> > > > >> >> - It has a mod clock and a bus clock.
> >> > > > >> >> - The mod clock must be at a fixed rate to generate
> >
> >signal.
> >
> >> > > > >> >Why?
> >> > > > >> 
> >> > > > >> It's experiment result by Jernej.
> >> > > > >> 
> >> > > > >> The clock rates in BSP kernel is also specially designed
> >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> >> > > > > 
> >> > > > > My experiments and search through BSP code showed that TVE
> >
> >seems to have
> >
> >> > > > > additional fixed predivider 8. So if you want to generate 27
> >
> >MHz clock,
> >
> >> > > > > unit has to be feed with 216 MHz.
> >> > > > > 
> >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
> >
> >bit low for
> >
> >> > > > > DE2,
> >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
> >
> >generate 216 MHz.
> >
> >> > > > > This clock is then divided by 8 internaly to get final 27
> >
> >MHz.
> >
> >> > > > > Please note that I don't have any hard evidence to support
> >
> >that, only
> >
> >> > > > > experimental data. However, only that explanation make sense
> >
> >to me.
> >
> >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
> >
> >both use 27 MHz
> >
> >> > > > > base clock. Further experiments are needed to check if there
> >
> >is any
> >
> >> > > > > possibility to have other resolutions by manipulating clocks
> >
> >and give
> >
> >> > > > > other proper settings. I plan to do that, but not in very
> >
> >near future.
> >
> >> > > > You only have composite video output, and those are the only 2
> >
> >standard
> >
> >> > > > resolutions that make any sense.
> >> > > 
> >> > > Right, other resolutions are for VGA.
> >> > > 
> >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
> >
> >think
> >
> >> > > that H3 TVE
> >> > > unit is something in between. R40 TVE has a setting to select "up
> >> > > sample".
> >> > 
> >> > That might be just another translation of oversampling :)
> >> > 
> >> > I didn't know it could be applied to composite signals though, but
> >
> >I
> >
> >> > guess this is just another analog signal after all.
> >> > 
> >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> >> > > driver on R40
> >> > > has this setting enabled only for PAL and NTSC and it is always
> >
> >216
> >
> >> > > MHz. I
> >> > > think that H3 may have this hardwired to 216 MHz and this would
> >
> >be
> >
> >> > > the reason
> >> > > why 216 MHz is needed.
> >> > > 
> >> > > Has anyone else any better explanation?
> >> > 
> >> > That's already a pretty good one.
> >> > 
> >> > Either way, wether this is upsampling, oversampling or just a
> >> > pre-divider, this can and should be dealt with in the mode_set
> >> > callback, and not in the probe.
> >> 
> >> I got a better idea -- let TVE driver have the CLK_TVE as an
> >> input and create a subclock output with divider 16, and feed this
> >> subclock to TCON lcd-ch1.
> >> 
> >> This is a model of the real hardware -- the clock divider is in
> >> TVE, not TCON.

If we are talking about HW divider, it is 8 (216 / 27 = 8).

Slightly offtopic, reason why DE2 is hardcoded to 432 might be that for 4K 
resolution you need at least 297 MHz. So next dividable frequency is taken 
(432 MHz). That way you can have 4K HDMI display and composite TV connected at 
the same time, although this sounds a bit weird.

Best regards,
Jernej

> >
> >That's definitely not a good representation of the hardware. There's
> >one clock, it goes to the TCON, period.
> 
> No, I still think it goes to the TVE as:
> 
> 1. it's named TVE in datasheet.
> 2. Generating signal with such a low resolution but such
> a high dotclock is not a good situation.
> 
> >However, the TV encoder has a constraint on that clock rate. This can
> >be easily implemented using a custom encoder state where you'd set the
> >multiplier to set on that clock, and the TCON will use it.
> >
> >Maxime
Maxime Ripard May 31, 2017, 6:43 p.m. UTC | #12
On Wed, May 24, 2017 at 04:25:46PM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2017年5月24日 GMT+08:00 下午3:30:19, Maxime Ripard <maxime.ripard@free-electrons.com> 写到:
> >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
> >> 在 2017-05-23 20:53,Maxime Ripard 写道:
> >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
> >> > > Hi,
> >> > > 
> >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
> >napisal(a):
> >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec
> ><jernej.skrabec@siol.net>
> >> > > wrote:
> >> > > > > Hi,
> >> > > > >
> >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
> >napisal(a):
> >> > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard
> ><maxime.ripard@free-
> >> > > > >
> >> > > > > electrons.com> 写到:
> >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
> >wrote:
> >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
> >earlier
> >> > > > >> >
> >> > > > >> >SoCs,
> >> > > > >> >
> >> > > > >> >> but with some different points about clocks:
> >> > > > >> >> - It has a mod clock and a bus clock.
> >> > > > >> >> - The mod clock must be at a fixed rate to generate
> >signal.
> >> > > > >> >
> >> > > > >> >Why?
> >> > > > >>
> >> > > > >> It's experiment result by Jernej.
> >> > > > >>
> >> > > > >> The clock rates in BSP kernel is also specially designed
> >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> >> > > > >
> >> > > > > My experiments and search through BSP code showed that TVE
> >seems to have
> >> > > > > additional fixed predivider 8. So if you want to generate 27
> >MHz clock,
> >> > > > > unit has to be feed with 216 MHz.
> >> > > > >
> >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
> >bit low for
> >> > > > > DE2,
> >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
> >generate 216 MHz.
> >> > > > > This clock is then divided by 8 internaly to get final 27
> >MHz.
> >> > > > >
> >> > > > > Please note that I don't have any hard evidence to support
> >that, only
> >> > > > > experimental data. However, only that explanation make sense
> >to me.
> >> > > > >
> >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
> >both use 27 MHz
> >> > > > > base clock. Further experiments are needed to check if there
> >is any
> >> > > > > possibility to have other resolutions by manipulating clocks
> >and give
> >> > > > > other proper settings. I plan to do that, but not in very
> >near future.
> >> > > >
> >> > > > You only have composite video output, and those are the only 2
> >standard
> >> > > > resolutions that make any sense.
> >> > > 
> >> > > Right, other resolutions are for VGA.
> >> > > 
> >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
> >think
> >> > > that H3 TVE
> >> > > unit is something in between. R40 TVE has a setting to select "up
> >> > > sample".
> >> > 
> >> > That might be just another translation of oversampling :)
> >> > 
> >> > I didn't know it could be applied to composite signals though, but
> >I
> >> > guess this is just another analog signal after all.
> >> > 
> >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> >> > > driver on R40
> >> > > has this setting enabled only for PAL and NTSC and it is always
> >216
> >> > > MHz. I
> >> > > think that H3 may have this hardwired to 216 MHz and this would
> >be
> >> > > the reason
> >> > > why 216 MHz is needed.
> >> > > 
> >> > > Has anyone else any better explanation?
> >> > 
> >> > That's already a pretty good one.
> >> > 
> >> > Either way, wether this is upsampling, oversampling or just a
> >> > pre-divider, this can and should be dealt with in the mode_set
> >> > callback, and not in the probe.
> >> 
> >> I got a better idea -- let TVE driver have the CLK_TVE as an
> >> input and create a subclock output with divider 16, and feed this
> >> subclock to TCON lcd-ch1.
> >> 
> >> This is a model of the real hardware -- the clock divider is in
> >> TVE, not TCON.
> >
> >That's definitely not a good representation of the hardware. There's
> >one clock, it goes to the TCON, period.
> 
> No, I still think it goes to the TVE as:
> 
> 1. it's named TVE in datasheet.

Feel free to come up with a better, more sensible explanation?

> 2. Generating signal with such a low resolution but such
> a high dotclock is not a good situation.

What? What do you mean? The pixel clock is 27MHz, I'm pretty sure we
agree on that.

Maxime
Icenowy Zheng June 1, 2017, 2:11 p.m. UTC | #13
在 2017-06-01 02:43,Maxime Ripard 写道:
> On Wed, May 24, 2017 at 04:25:46PM +0800, Icenowy Zheng wrote:
>> 
>> 
>> 于 2017年5月24日 GMT+08:00 下午3:30:19, Maxime Ripard 
>> <maxime.ripard@free-electrons.com> 写到:
>> >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
>> >> 在 2017-05-23 20:53,Maxime Ripard 写道:
>> >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
>> >> > > Hi,
>> >> > >
>> >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
>> >napisal(a):
>> >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec
>> ><jernej.skrabec@siol.net>
>> >> > > wrote:
>> >> > > > > Hi,
>> >> > > > >
>> >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
>> >napisal(a):
>> >> > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard
>> ><maxime.ripard@free-
>> >> > > > >
>> >> > > > > electrons.com> 写到:
>> >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
>> >wrote:
>> >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
>> >earlier
>> >> > > > >> >
>> >> > > > >> >SoCs,
>> >> > > > >> >
>> >> > > > >> >> but with some different points about clocks:
>> >> > > > >> >> - It has a mod clock and a bus clock.
>> >> > > > >> >> - The mod clock must be at a fixed rate to generate
>> >signal.
>> >> > > > >> >
>> >> > > > >> >Why?
>> >> > > > >>
>> >> > > > >> It's experiment result by Jernej.
>> >> > > > >>
>> >> > > > >> The clock rates in BSP kernel is also specially designed
>> >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> >> > > > >
>> >> > > > > My experiments and search through BSP code showed that TVE
>> >seems to have
>> >> > > > > additional fixed predivider 8. So if you want to generate 27
>> >MHz clock,
>> >> > > > > unit has to be feed with 216 MHz.
>> >> > > > >
>> >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
>> >bit low for
>> >> > > > > DE2,
>> >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
>> >generate 216 MHz.
>> >> > > > > This clock is then divided by 8 internaly to get final 27
>> >MHz.
>> >> > > > >
>> >> > > > > Please note that I don't have any hard evidence to support
>> >that, only
>> >> > > > > experimental data. However, only that explanation make sense
>> >to me.
>> >> > > > >
>> >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
>> >both use 27 MHz
>> >> > > > > base clock. Further experiments are needed to check if there
>> >is any
>> >> > > > > possibility to have other resolutions by manipulating clocks
>> >and give
>> >> > > > > other proper settings. I plan to do that, but not in very
>> >near future.
>> >> > > >
>> >> > > > You only have composite video output, and those are the only 2
>> >standard
>> >> > > > resolutions that make any sense.
>> >> > >
>> >> > > Right, other resolutions are for VGA.
>> >> > >
>> >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
>> >think
>> >> > > that H3 TVE
>> >> > > unit is something in between. R40 TVE has a setting to select "up
>> >> > > sample".
>> >> >
>> >> > That might be just another translation of oversampling :)
>> >> >
>> >> > I didn't know it could be applied to composite signals though, but
>> >I
>> >> > guess this is just another analog signal after all.
>> >> >
>> >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
>> >> > > driver on R40
>> >> > > has this setting enabled only for PAL and NTSC and it is always
>> >216
>> >> > > MHz. I
>> >> > > think that H3 may have this hardwired to 216 MHz and this would
>> >be
>> >> > > the reason
>> >> > > why 216 MHz is needed.
>> >> > >
>> >> > > Has anyone else any better explanation?
>> >> >
>> >> > That's already a pretty good one.
>> >> >
>> >> > Either way, wether this is upsampling, oversampling or just a
>> >> > pre-divider, this can and should be dealt with in the mode_set
>> >> > callback, and not in the probe.
>> >>
>> >> I got a better idea -- let TVE driver have the CLK_TVE as an
>> >> input and create a subclock output with divider 16, and feed this
>> >> subclock to TCON lcd-ch1.
>> >>
>> >> This is a model of the real hardware -- the clock divider is in
>> >> TVE, not TCON.
>> >
>> >That's definitely not a good representation of the hardware. There's
>> >one clock, it goes to the TCON, period.
>> 
>> No, I still think it goes to the TVE as:
>> 
>> 1. it's named TVE in datasheet.
> 
> Feel free to come up with a better, more sensible explanation?
> 
>> 2. Generating signal with such a low resolution but such
>> a high dotclock is not a good situation.
> 
> What? What do you mean? The pixel clock is 27MHz, I'm pretty sure we
> agree on that.

Yes, so I don't think the TCON code should set the clock to any value
rather than 27MHz.

So we should create a clock with divider in TVE, and feed it to TCON.

> 
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Maxime Ripard June 2, 2017, 10:21 p.m. UTC | #14
On Thu, Jun 01, 2017 at 10:11:14PM +0800, icenowy@aosc.io wrote:
> 在 2017-06-01 02:43,Maxime Ripard 写道:
> > On Wed, May 24, 2017 at 04:25:46PM +0800, Icenowy Zheng wrote:
> > > 
> > > 
> > > 于 2017年5月24日 GMT+08:00 下午3:30:19, Maxime Ripard
> > > <maxime.ripard@free-electrons.com> 写到:
> > > >On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
> > > >> 在 2017-05-23 20:53,Maxime Ripard 写道:
> > > >> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
> > > >> > > Hi,
> > > >> > >
> > > >> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai
> > > >napisal(a):
> > > >> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec
> > > ><jernej.skrabec@siol.net>
> > > >> > > wrote:
> > > >> > > > > Hi,
> > > >> > > > >
> > > >> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng
> > > >napisal(a):
> > > >> > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard
> > > ><maxime.ripard@free-
> > > >> > > > >
> > > >> > > > > electrons.com> 写到:
> > > >> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng
> > > >wrote:
> > > >> > > > >> >> Allwinner H3 features a TV encoder similar to the one in
> > > >earlier
> > > >> > > > >> >
> > > >> > > > >> >SoCs,
> > > >> > > > >> >
> > > >> > > > >> >> but with some different points about clocks:
> > > >> > > > >> >> - It has a mod clock and a bus clock.
> > > >> > > > >> >> - The mod clock must be at a fixed rate to generate
> > > >signal.
> > > >> > > > >> >
> > > >> > > > >> >Why?
> > > >> > > > >>
> > > >> > > > >> It's experiment result by Jernej.
> > > >> > > > >>
> > > >> > > > >> The clock rates in BSP kernel is also specially designed
> > > >> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > >> > > > >
> > > >> > > > > My experiments and search through BSP code showed that TVE
> > > >seems to have
> > > >> > > > > additional fixed predivider 8. So if you want to generate 27
> > > >MHz clock,
> > > >> > > > > unit has to be feed with 216 MHz.
> > > >> > > > >
> > > >> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a
> > > >bit low for
> > > >> > > > > DE2,
> > > >> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to
> > > >generate 216 MHz.
> > > >> > > > > This clock is then divided by 8 internaly to get final 27
> > > >MHz.
> > > >> > > > >
> > > >> > > > > Please note that I don't have any hard evidence to support
> > > >that, only
> > > >> > > > > experimental data. However, only that explanation make sense
> > > >to me.
> > > >> > > > >
> > > >> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which
> > > >both use 27 MHz
> > > >> > > > > base clock. Further experiments are needed to check if there
> > > >is any
> > > >> > > > > possibility to have other resolutions by manipulating clocks
> > > >and give
> > > >> > > > > other proper settings. I plan to do that, but not in very
> > > >near future.
> > > >> > > >
> > > >> > > > You only have composite video output, and those are the only 2
> > > >standard
> > > >> > > > resolutions that make any sense.
> > > >> > >
> > > >> > > Right, other resolutions are for VGA.
> > > >> > >
> > > >> > > Anyway, I did some more digging in A10 and R40 datasheets. I
> > > >think
> > > >> > > that H3 TVE
> > > >> > > unit is something in between. R40 TVE has a setting to select "up
> > > >> > > sample".
> > > >> >
> > > >> > That might be just another translation of oversampling :)
> > > >> >
> > > >> > I didn't know it could be applied to composite signals though, but
> > > >I
> > > >> > guess this is just another analog signal after all.
> > > >> >
> > > >> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> > > >> > > driver on R40
> > > >> > > has this setting enabled only for PAL and NTSC and it is always
> > > >216
> > > >> > > MHz. I
> > > >> > > think that H3 may have this hardwired to 216 MHz and this would
> > > >be
> > > >> > > the reason
> > > >> > > why 216 MHz is needed.
> > > >> > >
> > > >> > > Has anyone else any better explanation?
> > > >> >
> > > >> > That's already a pretty good one.
> > > >> >
> > > >> > Either way, wether this is upsampling, oversampling or just a
> > > >> > pre-divider, this can and should be dealt with in the mode_set
> > > >> > callback, and not in the probe.
> > > >>
> > > >> I got a better idea -- let TVE driver have the CLK_TVE as an
> > > >> input and create a subclock output with divider 16, and feed this
> > > >> subclock to TCON lcd-ch1.
> > > >>
> > > >> This is a model of the real hardware -- the clock divider is in
> > > >> TVE, not TCON.
> > > >
> > > >That's definitely not a good representation of the hardware. There's
> > > >one clock, it goes to the TCON, period.
> > > 
> > > No, I still think it goes to the TVE as:
> > > 
> > > 1. it's named TVE in datasheet.
> > 
> > Feel free to come up with a better, more sensible explanation?
> > 
> > > 2. Generating signal with such a low resolution but such
> > > a high dotclock is not a good situation.
> > 
> > What? What do you mean? The pixel clock is 27MHz, I'm pretty sure we
> > agree on that.
> 
> Yes, so I don't think the TCON code should set the clock to any value
> rather than 27MHz.

The pixel clock that matters is what is output on the TV
connector. The intermediate pixel clock don't matter, and in this
case, it seems to be clearly the case that the TCON needs a higher
clock, then let's give it a higher clock.

> So we should create a clock with divider in TVE, and feed it to TCON.

I'm sorry, but this still doesn't make much sense. The clock is
provided by the TCON, and there's a divider in the TVE, it's as simple
as that, unless you can provide some other meaningful explanation.

That the TV encoder feeds a clock that would drive the TCON is already
suspicious, and all the precedent designs indicate that this is not
what is going on. Both the explanation and the design is much simpler
that way.

Sometimes the Occam's razor is a better justification than a label on
a datasheet.

Maxime
Icenowy Zheng June 4, 2017, 2:29 p.m. UTC | #15
在 2017-05-24 15:30,Maxime Ripard 写道:
> On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
>> 在 2017-05-23 20:53,Maxime Ripard 写道:
>> > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
>> > > Hi,
>> > >
>> > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
>> > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec <jernej.skrabec@siol.net>
>> > > wrote:
>> > > > > Hi,
>> > > > >
>> > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
>> > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
>> > > > >
>> > > > > electrons.com> 写到:
>> > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> > > > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
>> > > > >> >
>> > > > >> >SoCs,
>> > > > >> >
>> > > > >> >> but with some different points about clocks:
>> > > > >> >> - It has a mod clock and a bus clock.
>> > > > >> >> - The mod clock must be at a fixed rate to generate signal.
>> > > > >> >
>> > > > >> >Why?
>> > > > >>
>> > > > >> It's experiment result by Jernej.
>> > > > >>
>> > > > >> The clock rates in BSP kernel is also specially designed
>> > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
>> > > > >
>> > > > > My experiments and search through BSP code showed that TVE seems to have
>> > > > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
>> > > > > unit has to be feed with 216 MHz.
>> > > > >
>> > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
>> > > > > DE2,
>> > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
>> > > > > This clock is then divided by 8 internaly to get final 27 MHz.
>> > > > >
>> > > > > Please note that I don't have any hard evidence to support that, only
>> > > > > experimental data. However, only that explanation make sense to me.
>> > > > >
>> > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
>> > > > > base clock. Further experiments are needed to check if there is any
>> > > > > possibility to have other resolutions by manipulating clocks and give
>> > > > > other proper settings. I plan to do that, but not in very near future.
>> > > >
>> > > > You only have composite video output, and those are the only 2 standard
>> > > > resolutions that make any sense.
>> > >
>> > > Right, other resolutions are for VGA.
>> > >
>> > > Anyway, I did some more digging in A10 and R40 datasheets. I think
>> > > that H3 TVE
>> > > unit is something in between. R40 TVE has a setting to select "up
>> > > sample".
>> >
>> > That might be just another translation of oversampling :)
>> >
>> > I didn't know it could be applied to composite signals though, but I
>> > guess this is just another analog signal after all.
>> >
>> > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
>> > > driver on R40
>> > > has this setting enabled only for PAL and NTSC and it is always 216
>> > > MHz. I
>> > > think that H3 may have this hardwired to 216 MHz and this would be
>> > > the reason
>> > > why 216 MHz is needed.
>> > >
>> > > Has anyone else any better explanation?
>> >
>> > That's already a pretty good one.
>> >
>> > Either way, wether this is upsampling, oversampling or just a
>> > pre-divider, this can and should be dealt with in the mode_set
>> > callback, and not in the probe.
>> 
>> I got a better idea -- let TVE driver have the CLK_TVE as an
>> input and create a subclock output with divider 16, and feed this
>> subclock to TCON lcd-ch1.
>> 
>> This is a model of the real hardware -- the clock divider is in
>> TVE, not TCON.
> 
> That's definitely not a good representation of the hardware. There's
> one clock, it goes to the TCON, period.
> 
> However, the TV encoder has a constraint on that clock rate. This can
> be easily implemented using a custom encoder state where you'd set the
> multiplier to set on that clock, and the TCON will use it.

P.S. how to do such a custom state?

Should I do the multiplying in sun4i_tv_mode_to_drm_mode?

> 
> Maxime
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Maxime Ripard June 7, 2017, 7:58 a.m. UTC | #16
On Sun, Jun 04, 2017 at 10:29:29PM +0800, icenowy@aosc.io wrote:
> 在 2017-05-24 15:30,Maxime Ripard 写道:
> > On Tue, May 23, 2017 at 09:00:59PM +0800, icenowy@aosc.io wrote:
> > > 在 2017-05-23 20:53,Maxime Ripard 写道:
> > > > On Mon, May 22, 2017 at 07:55:56PM +0200, Jernej Škrabec wrote:
> > > > > Hi,
> > > > >
> > > > > Dne sobota, 20. maj 2017 ob 03:37:53 CEST je Chen-Yu Tsai napisal(a):
> > > > > > On Sat, May 20, 2017 at 2:23 AM, Jernej Škrabec <jernej.skrabec@siol.net>
> > > > > wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> > > > > > >> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard <maxime.ripard@free-
> > > > > > >
> > > > > > > electrons.com> 写到:
> > > > > > >> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> > > > > > >> >> Allwinner H3 features a TV encoder similar to the one in earlier
> > > > > > >> >
> > > > > > >> >SoCs,
> > > > > > >> >
> > > > > > >> >> but with some different points about clocks:
> > > > > > >> >> - It has a mod clock and a bus clock.
> > > > > > >> >> - The mod clock must be at a fixed rate to generate signal.
> > > > > > >> >
> > > > > > >> >Why?
> > > > > > >>
> > > > > > >> It's experiment result by Jernej.
> > > > > > >>
> > > > > > >> The clock rates in BSP kernel is also specially designed
> > > > > > >> (PLL_DE at 432MHz) in order to be able to feed the TVE.
> > > > > > >
> > > > > > > My experiments and search through BSP code showed that TVE seems to have
> > > > > > > additional fixed predivider 8. So if you want to generate 27 MHz clock,
> > > > > > > unit has to be feed with 216 MHz.
> > > > > > >
> > > > > > > TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for
> > > > > > > DE2,
> > > > > > > BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz.
> > > > > > > This clock is then divided by 8 internaly to get final 27 MHz.
> > > > > > >
> > > > > > > Please note that I don't have any hard evidence to support that, only
> > > > > > > experimental data. However, only that explanation make sense to me.
> > > > > > >
> > > > > > > BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz
> > > > > > > base clock. Further experiments are needed to check if there is any
> > > > > > > possibility to have other resolutions by manipulating clocks and give
> > > > > > > other proper settings. I plan to do that, but not in very near future.
> > > > > >
> > > > > > You only have composite video output, and those are the only 2 standard
> > > > > > resolutions that make any sense.
> > > > >
> > > > > Right, other resolutions are for VGA.
> > > > >
> > > > > Anyway, I did some more digging in A10 and R40 datasheets. I think
> > > > > that H3 TVE
> > > > > unit is something in between. R40 TVE has a setting to select "up
> > > > > sample".
> > > >
> > > > That might be just another translation of oversampling :)
> > > >
> > > > I didn't know it could be applied to composite signals though, but I
> > > > guess this is just another analog signal after all.
> > > >
> > > > > Possible settings are 27 MHz, 54 MHz, 108 MHz and 216 MHz. BSP
> > > > > driver on R40
> > > > > has this setting enabled only for PAL and NTSC and it is always 216
> > > > > MHz. I
> > > > > think that H3 may have this hardwired to 216 MHz and this would be
> > > > > the reason
> > > > > why 216 MHz is needed.
> > > > >
> > > > > Has anyone else any better explanation?
> > > >
> > > > That's already a pretty good one.
> > > >
> > > > Either way, wether this is upsampling, oversampling or just a
> > > > pre-divider, this can and should be dealt with in the mode_set
> > > > callback, and not in the probe.
> > > 
> > > I got a better idea -- let TVE driver have the CLK_TVE as an
> > > input and create a subclock output with divider 16, and feed this
> > > subclock to TCON lcd-ch1.
> > > 
> > > This is a model of the real hardware -- the clock divider is in
> > > TVE, not TCON.
> > 
> > That's definitely not a good representation of the hardware. There's
> > one clock, it goes to the TCON, period.
> > 
> > However, the TV encoder has a constraint on that clock rate. This can
> > be easily implemented using a custom encoder state where you'd set the
> > multiplier to set on that clock, and the TCON will use it.
> 
> P.S. how to do such a custom state?
> 
> Should I do the multiplying in sun4i_tv_mode_to_drm_mode?

You have an example of that in ebd14afe8fa7, but it basically boils
down to:
  - Create a structure subclassing drm_connector_state
  - Adding your own reset and atomic_duplicate_state callback
  - In the atomic_check, change the state to set the pre-divider, and
    then use it in TCON's mode_set.

Maxime
diff mbox

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index a9cad00d4ee8..c9943103f499 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -13,6 +13,7 @@ 
 #include <linux/clk.h>
 #include <linux/component.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
@@ -169,14 +170,23 @@  struct tv_mode {
 	const struct resync_parameters	*resync_params;
 };
 
+struct sun4i_tv_quirks {
+	bool has_mod_clk;
+	bool fixed_clock;
+	unsigned long fixed_clock_rate;
+};
+
 struct sun4i_tv {
 	struct drm_connector	connector;
 	struct drm_encoder	encoder;
 
 	struct clk		*clk;
+	struct clk		*mod_clk;
 	struct regmap		*regs;
 	struct reset_control	*reset;
 
+	const struct sun4i_tv_quirks *quirks;
+
 	struct sun4i_drv	*drv;
 };
 
@@ -578,6 +588,10 @@  static int sun4i_tv_bind(struct device *dev, struct device *master,
 	tv->drv = drv;
 	dev_set_drvdata(dev, tv);
 
+	tv->quirks = of_device_get_match_data(dev);
+	if (!tv->quirks)
+		return -EINVAL;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(regs)) {
@@ -604,7 +618,10 @@  static int sun4i_tv_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	tv->clk = devm_clk_get(dev, NULL);
+	if (tv->quirks->has_mod_clk)
+		tv->clk = devm_clk_get(dev, "bus");
+	else
+		tv->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(tv->clk)) {
 		dev_err(dev, "Couldn't get the TV encoder clock\n");
 		ret = PTR_ERR(tv->clk);
@@ -612,6 +629,26 @@  static int sun4i_tv_bind(struct device *dev, struct device *master,
 	}
 	clk_prepare_enable(tv->clk);
 
+	if (tv->quirks->has_mod_clk) {
+		tv->mod_clk = devm_clk_get(dev, "mod");
+		if (IS_ERR(tv->mod_clk)) {
+			dev_err(dev, "Couldn't get the TV encoder mod clock\n");
+			ret = PTR_ERR(tv->mod_clk);
+			goto err_disable_clk;
+		};
+
+		if (tv->quirks->fixed_clock) {
+			ret = clk_set_rate(tv->mod_clk,
+					   tv->quirks->fixed_clock_rate);
+			if (ret) {
+				dev_err(dev, "Couldn't set TV encoder mod clock rate\n");
+				goto err_disable_clk;
+			}
+		}
+
+		clk_prepare_enable(tv->mod_clk);
+	}
+
 	drm_encoder_helper_add(&tv->encoder,
 			       &sun4i_tv_helper_funcs);
 	ret = drm_encoder_init(drm,
@@ -621,14 +658,14 @@  static int sun4i_tv_bind(struct device *dev, struct device *master,
 			       NULL);
 	if (ret) {
 		dev_err(dev, "Couldn't initialise the TV encoder\n");
-		goto err_disable_clk;
+		goto err_disable_mod_clk;
 	}
 
 	tv->encoder.possible_crtcs = drm_of_find_possible_crtcs(drm,
 								dev->of_node);
 	if (!tv->encoder.possible_crtcs) {
 		ret = -EPROBE_DEFER;
-		goto err_disable_clk;
+		goto err_disable_mod_clk;
 	}
 
 	drm_connector_helper_add(&tv->connector,
@@ -649,6 +686,9 @@  static int sun4i_tv_bind(struct device *dev, struct device *master,
 
 err_cleanup_connector:
 	drm_encoder_cleanup(&tv->encoder);
+err_disable_mod_clk:
+	if (tv->quirks->has_mod_clk)
+		clk_disable_unprepare(tv->mod_clk);
 err_disable_clk:
 	clk_disable_unprepare(tv->clk);
 err_assert_reset:
@@ -683,8 +723,25 @@  static int sun4i_tv_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct sun4i_tv_quirks sun4i_a10_tv_quirks = {
+	/* Nothing special */
+};
+
+static const struct sun4i_tv_quirks sun8i_h3_tv_quirks = {
+	.has_mod_clk = true,
+	.fixed_clock = true,
+	.fixed_clock_rate = 216000000UL,
+};
+
 static const struct of_device_id sun4i_tv_of_table[] = {
-	{ .compatible = "allwinner,sun4i-a10-tv-encoder" },
+	{
+		.compatible = "allwinner,sun4i-a10-tv-encoder",
+		.data = &sun4i_a10_tv_quirks,
+	},
+	{
+		.compatible = "allwinner,sun8i-h3-tv-encoder",
+		.data = &sun8i_h3_tv_quirks,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun4i_tv_of_table);