diff mbox

[U-Boot,v2,4/4] sunxi: video: Add H3/H5 TV out driver

Message ID 20170519154118.28651-5-jernej.skrabec@siol.net
State Deferred
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Jernej Škrabec May 19, 2017, 3:41 p.m. UTC
This commit adds support for TV (composite) output.

Because there is no mechanism to select TV standard, PAL is hardcoded.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---

 arch/arm/include/asm/arch-sunxi/tve.h |  17 +++-
 drivers/video/sunxi/Makefile          |   2 +-
 drivers/video/sunxi/sunxi_tve.c       | 156 ++++++++++++++++++++++++++++++++++
 drivers/video/sunxi/tve_common.c      |   6 +-
 4 files changed, 176 insertions(+), 5 deletions(-)
 create mode 100644 drivers/video/sunxi/sunxi_tve.c

Comments

Maxime Ripard May 22, 2017, 7:35 a.m. UTC | #1
On Fri, May 19, 2017 at 05:41:17PM +0200, Jernej Skrabec wrote:
> This commit adds support for TV (composite) output.
> 
> Because there is no mechanism to select TV standard, PAL is
> hardcoded.

I'd rather use a consistent mechanism with the old driver (even if we
only support PAL right now and reject any other option), and using
composite-pal as monitor.

Maxime
Jernej Škrabec May 22, 2017, 6:49 p.m. UTC | #2
Hi Maxime,

Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):
> On Fri, May 19, 2017 at 05:41:17PM +0200, Jernej Skrabec wrote:
> > This commit adds support for TV (composite) output.
> > 
> > Because there is no mechanism to select TV standard, PAL is
> > hardcoded.
> 
> I'd rather use a consistent mechanism with the old driver (even if we
> only support PAL right now and reject any other option), and using
> composite-pal as monitor.
> 

I have few arguments against that:

1. Code for parsing that env variable is in videomodes.[c|h], which is clearly 
a part of an older video framework (ctfb). I didn't want to include any legacy 
code.

2. Even if this code is used for parsing, it would bring a lot of confusion. 
For now, we can say that docs/README.video does not apply to H3 and newer 
SoCs. If we implement this only partially, we would need to describe in 
details which of each setting is honored with the new driver and which not. 
Even then, a lot of users would skip that description and complain anyway.

3. If anything is done in this direction, I think that it is better to extend 
DM video framework so other drivers would benefit from that work too.

Best regards,
Jernej
Maxime Ripard May 23, 2017, 8:22 p.m. UTC | #3
Hi Jernej,

On Mon, May 22, 2017 at 08:49:57PM +0200, Jernej Škrabec wrote:
> Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):
> > On Fri, May 19, 2017 at 05:41:17PM +0200, Jernej Skrabec wrote:
> > > This commit adds support for TV (composite) output.
> > > 
> > > Because there is no mechanism to select TV standard, PAL is
> > > hardcoded.
> > 
> > I'd rather use a consistent mechanism with the old driver (even if we
> > only support PAL right now and reject any other option), and using
> > composite-pal as monitor.
> > 
> 
> I have few arguments against that:
> 
> 1. Code for parsing that env variable is in videomodes.[c|h], which is clearly 
> a part of an older video framework (ctfb). I didn't want to include any legacy 
> code.
> 
> 2. Even if this code is used for parsing, it would bring a lot of confusion. 
> For now, we can say that docs/README.video does not apply to H3 and newer 
> SoCs. If we implement this only partially, we would need to describe in 
> details which of each setting is honored with the new driver and which not. 
> Even then, a lot of users would skip that description and complain anyway.

The issue with this, and we've been bitten very hard on this one with
the CHIP, is that you don't really have a clear majority on that
one. If you support only PAL, half the world will be left out, and
same thing with NTSC (for some reason, we never needed to support the
less common ones like PAL-M or NTSC-J, but that just might be because
it never really sold that well in those countries, I don't have any
numbers on that).

The point is, if you just hardcode PAL for now, you will have half
your users complain, and then, when we will introduce NTSC support
eventually, we'll have to introduce some mechanism to switch between
the two, then we'll probably break the behaviour our users relied on
before, making the other half of our users pissed.

I'm not sure this is something we should just discard, or at least the
second part. Having the selection mechanism in place, even if we don't
support all the settings and just report an error in the logs in such
a case address the latter issue.

You'll also need to address how to setup the overscan, since this is
really something you want to have very quick.

> 3. If anything is done in this direction, I think that it is better
> to extend DM video framework so other drivers would benefit from
> that work too.

That makes sense, but again, this is a pre-requisite for me. And it's
not that hard to support the video modelines with a device model
driver, Linux does it, and you have a string identifier at the
beginning of it. It just has to be deterministic, but I don't think
this is an issue with U-Boot's DM.

Maxime
Jernej Škrabec May 24, 2017, 3:34 p.m. UTC | #4
Hi,

Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):
> Hi Jernej,
> 
> On Mon, May 22, 2017 at 08:49:57PM +0200, Jernej Škrabec wrote:
> > Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):
> > > On Fri, May 19, 2017 at 05:41:17PM +0200, Jernej Skrabec wrote:
> > > > This commit adds support for TV (composite) output.
> > > > 
> > > > Because there is no mechanism to select TV standard, PAL is
> > > > hardcoded.
> > > 
> > > I'd rather use a consistent mechanism with the old driver (even if we
> > > only support PAL right now and reject any other option), and using
> > > composite-pal as monitor.
> > 
> > I have few arguments against that:
> > 
> > 1. Code for parsing that env variable is in videomodes.[c|h], which is
> > clearly a part of an older video framework (ctfb). I didn't want to
> > include any legacy code.
> > 
> > 2. Even if this code is used for parsing, it would bring a lot of
> > confusion. For now, we can say that docs/README.video does not apply to
> > H3 and newer SoCs. If we implement this only partially, we would need to
> > describe in details which of each setting is honored with the new driver
> > and which not. Even then, a lot of users would skip that description and
> > complain anyway.
> The issue with this, and we've been bitten very hard on this one with
> the CHIP, is that you don't really have a clear majority on that
> one. If you support only PAL, half the world will be left out, and
> same thing with NTSC (for some reason, we never needed to support the
> less common ones like PAL-M or NTSC-J, but that just might be because
> it never really sold that well in those countries, I don't have any
> numbers on that).
> 
> The point is, if you just hardcode PAL for now, you will have half
> your users complain, and then, when we will introduce NTSC support
> eventually, we'll have to introduce some mechanism to switch between
> the two, then we'll probably break the behaviour our users relied on
> before, making the other half of our users pissed.
> 
> I'm not sure this is something we should just discard, or at least the
> second part. Having the selection mechanism in place, even if we don't
> support all the settings and just report an error in the logs in such
> a case address the latter issue.
> 
> You'll also need to address how to setup the overscan, since this is
> really something you want to have very quick.

Ok, I'm prepared to tackle this. Do you think it is worth to delay driver 
merge?

> 
> > 3. If anything is done in this direction, I think that it is better
> > to extend DM video framework so other drivers would benefit from
> > that work too.
> 
> That makes sense, but again, this is a pre-requisite for me. And it's
> not that hard to support the video modelines with a device model
> driver, Linux does it, and you have a string identifier at the
> beginning of it. It just has to be deterministic, but I don't think
> this is an issue with U-Boot's DM.
> 

Ok, so how do you think we should implement this? If only composite modes are 
supported in the string, someone might try to set hdmi monitor. Should we just 
ignore such settings and maybe print a warning?

Also the question for Simon, how should I merge code for parsing video string 
from drivers/video/videomodes.c to DM video in a way to be useful to most 
drivers?

I guess this is the first time to support analog video output in DM video 
driver and there are some things missing such as a way to select TV standard 
and define overscan.

Or alternatively, I could make just quick edit to sunxi_tve.c driver and 
directly use old functions as they are. That way we could get something 
working very quickly, but I don't like much such approach.

Best regards,
Jernej
Maxime Ripard May 30, 2017, 8:41 p.m. UTC | #5
On Wed, May 24, 2017 at 05:34:52PM +0200, Jernej Škrabec wrote:
> Hi,
> 
> Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):
> > Hi Jernej,
> > 
> > On Mon, May 22, 2017 at 08:49:57PM +0200, Jernej Škrabec wrote:
> > > Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):
> > > > On Fri, May 19, 2017 at 05:41:17PM +0200, Jernej Skrabec wrote:
> > > > > This commit adds support for TV (composite) output.
> > > > > 
> > > > > Because there is no mechanism to select TV standard, PAL is
> > > > > hardcoded.
> > > > 
> > > > I'd rather use a consistent mechanism with the old driver (even if we
> > > > only support PAL right now and reject any other option), and using
> > > > composite-pal as monitor.
> > > 
> > > I have few arguments against that:
> > > 
> > > 1. Code for parsing that env variable is in videomodes.[c|h], which is
> > > clearly a part of an older video framework (ctfb). I didn't want to
> > > include any legacy code.
> > > 
> > > 2. Even if this code is used for parsing, it would bring a lot of
> > > confusion. For now, we can say that docs/README.video does not apply to
> > > H3 and newer SoCs. If we implement this only partially, we would need to
> > > describe in details which of each setting is honored with the new driver
> > > and which not. Even then, a lot of users would skip that description and
> > > complain anyway.
> > The issue with this, and we've been bitten very hard on this one with
> > the CHIP, is that you don't really have a clear majority on that
> > one. If you support only PAL, half the world will be left out, and
> > same thing with NTSC (for some reason, we never needed to support the
> > less common ones like PAL-M or NTSC-J, but that just might be because
> > it never really sold that well in those countries, I don't have any
> > numbers on that).
> > 
> > The point is, if you just hardcode PAL for now, you will have half
> > your users complain, and then, when we will introduce NTSC support
> > eventually, we'll have to introduce some mechanism to switch between
> > the two, then we'll probably break the behaviour our users relied on
> > before, making the other half of our users pissed.
> > 
> > I'm not sure this is something we should just discard, or at least the
> > second part. Having the selection mechanism in place, even if we don't
> > support all the settings and just report an error in the logs in such
> > a case address the latter issue.
> > 
> > You'll also need to address how to setup the overscan, since this is
> > really something you want to have very quick.
> 
> Ok, I'm prepared to tackle this. Do you think it is worth to delay driver 
> merge?

Not per se, but that should definitely be part of the same release, so
if you can make it by then, then we can merge that right now, and
merge the rest later. If you can't, then we'll have to postpone it a
bit.

> > > 3. If anything is done in this direction, I think that it is better
> > > to extend DM video framework so other drivers would benefit from
> > > that work too.
> > 
> > That makes sense, but again, this is a pre-requisite for me. And it's
> > not that hard to support the video modelines with a device model
> > driver, Linux does it, and you have a string identifier at the
> > beginning of it. It just has to be deterministic, but I don't think
> > this is an issue with U-Boot's DM.
> 
> Ok, so how do you think we should implement this? If only composite
> modes are supported in the string, someone might try to set hdmi
> monitor. Should we just ignore such settings and maybe print a
> warning?

I'm not sure it was your question, but I would just reject any
improper configuration, and not display anything in such a case.

> Also the question for Simon, how should I merge code for parsing
> video string from drivers/video/videomodes.c to DM video in a way to
> be useful to most drivers?
> 
> I guess this is the first time to support analog video output in DM
> video driver and there are some things missing such as a way to
> select TV standard and define overscan.

I think it can be done in the same way linux does it here:
http://elixir.free-electrons.com/linux/latest/source/drivers/video/fbdev/core/fb_cmdline.c#L35

You basically check first that there is a string matching what your
driver expects, and then get the options.

We could imagine having some extra function to parse that string and
give back a structure filled with whatever was set in that command
line. We could imagine having the common custom properties to be
parsed at that same time.

> Or alternatively, I could make just quick edit to sunxi_tve.c driver
> and directly use old functions as they are. That way we could get
> something working very quickly, but I don't like much such approach.

That's another approach, I'm fine with it as a temporary measure, but
I'm not sure how Simon feels about it.

Maxime
Simon Glass June 1, 2017, 3:11 a.m. UTC | #6
Hi Maxime,

On 30 May 2017 at 14:41, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> On Wed, May 24, 2017 at 05:34:52PM +0200, Jernej Škrabec wrote:
>> Hi,
>>
>> Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):
>> > Hi Jernej,
>> >
>> > On Mon, May 22, 2017 at 08:49:57PM +0200, Jernej Škrabec wrote:
>> > > Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):
>> > > > On Fri, May 19, 2017 at 05:41:17PM +0200, Jernej Skrabec wrote:
>> > > > > This commit adds support for TV (composite) output.
>> > > > >
>> > > > > Because there is no mechanism to select TV standard, PAL is
>> > > > > hardcoded.
>> > > >
>> > > > I'd rather use a consistent mechanism with the old driver (even if we
>> > > > only support PAL right now and reject any other option), and using
>> > > > composite-pal as monitor.
>> > >
>> > > I have few arguments against that:
>> > >
>> > > 1. Code for parsing that env variable is in videomodes.[c|h], which is
>> > > clearly a part of an older video framework (ctfb). I didn't want to
>> > > include any legacy code.
>> > >
>> > > 2. Even if this code is used for parsing, it would bring a lot of
>> > > confusion. For now, we can say that docs/README.video does not apply to
>> > > H3 and newer SoCs. If we implement this only partially, we would need to
>> > > describe in details which of each setting is honored with the new driver
>> > > and which not. Even then, a lot of users would skip that description and
>> > > complain anyway.
>> > The issue with this, and we've been bitten very hard on this one with
>> > the CHIP, is that you don't really have a clear majority on that
>> > one. If you support only PAL, half the world will be left out, and
>> > same thing with NTSC (for some reason, we never needed to support the
>> > less common ones like PAL-M or NTSC-J, but that just might be because
>> > it never really sold that well in those countries, I don't have any
>> > numbers on that).
>> >
>> > The point is, if you just hardcode PAL for now, you will have half
>> > your users complain, and then, when we will introduce NTSC support
>> > eventually, we'll have to introduce some mechanism to switch between
>> > the two, then we'll probably break the behaviour our users relied on
>> > before, making the other half of our users pissed.
>> >
>> > I'm not sure this is something we should just discard, or at least the
>> > second part. Having the selection mechanism in place, even if we don't
>> > support all the settings and just report an error in the logs in such
>> > a case address the latter issue.
>> >
>> > You'll also need to address how to setup the overscan, since this is
>> > really something you want to have very quick.
>>
>> Ok, I'm prepared to tackle this. Do you think it is worth to delay driver
>> merge?
>
> Not per se, but that should definitely be part of the same release, so
> if you can make it by then, then we can merge that right now, and
> merge the rest later. If you can't, then we'll have to postpone it a
> bit.
>
>> > > 3. If anything is done in this direction, I think that it is better
>> > > to extend DM video framework so other drivers would benefit from
>> > > that work too.
>> >
>> > That makes sense, but again, this is a pre-requisite for me. And it's
>> > not that hard to support the video modelines with a device model
>> > driver, Linux does it, and you have a string identifier at the
>> > beginning of it. It just has to be deterministic, but I don't think
>> > this is an issue with U-Boot's DM.
>>
>> Ok, so how do you think we should implement this? If only composite
>> modes are supported in the string, someone might try to set hdmi
>> monitor. Should we just ignore such settings and maybe print a
>> warning?
>
> I'm not sure it was your question, but I would just reject any
> improper configuration, and not display anything in such a case.
>
>> Also the question for Simon, how should I merge code for parsing
>> video string from drivers/video/videomodes.c to DM video in a way to
>> be useful to most drivers?
>>
>> I guess this is the first time to support analog video output in DM
>> video driver and there are some things missing such as a way to
>> select TV standard and define overscan.
>
> I think it can be done in the same way linux does it here:
> http://elixir.free-electrons.com/linux/latest/source/drivers/video/fbdev/core/fb_cmdline.c#L35
>
> You basically check first that there is a string matching what your
> driver expects, and then get the options.
>
> We could imagine having some extra function to parse that string and
> give back a structure filled with whatever was set in that command
> line. We could imagine having the common custom properties to be
> parsed at that same time.
>
>> Or alternatively, I could make just quick edit to sunxi_tve.c driver
>> and directly use old functions as they are. That way we could get
>> something working very quickly, but I don't like much such approach.
>
> That's another approach, I'm fine with it as a temporary measure, but
> I'm not sure how Simon feels about it.

Well how about a function that decodes the string into a struct? That
function could be called by any drivers that need it.

Regards,
Simon


>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Maxime Ripard June 1, 2017, 7:38 a.m. UTC | #7
On Wed, May 31, 2017 at 09:11:15PM -0600, Simon Glass wrote:
> Hi Maxime,
> 
> On 30 May 2017 at 14:41, Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > On Wed, May 24, 2017 at 05:34:52PM +0200, Jernej Škrabec wrote:
> >> Hi,
> >>
> >> Dne torek, 23. maj 2017 ob 22:22:14 CEST je Maxime Ripard napisal(a):
> >> > Hi Jernej,
> >> >
> >> > On Mon, May 22, 2017 at 08:49:57PM +0200, Jernej Škrabec wrote:
> >> > > Dne ponedeljek, 22. maj 2017 ob 09:35:56 CEST je Maxime Ripard napisal(a):
> >> > > > On Fri, May 19, 2017 at 05:41:17PM +0200, Jernej Skrabec wrote:
> >> > > > > This commit adds support for TV (composite) output.
> >> > > > >
> >> > > > > Because there is no mechanism to select TV standard, PAL is
> >> > > > > hardcoded.
> >> > > >
> >> > > > I'd rather use a consistent mechanism with the old driver (even if we
> >> > > > only support PAL right now and reject any other option), and using
> >> > > > composite-pal as monitor.
> >> > >
> >> > > I have few arguments against that:
> >> > >
> >> > > 1. Code for parsing that env variable is in videomodes.[c|h], which is
> >> > > clearly a part of an older video framework (ctfb). I didn't want to
> >> > > include any legacy code.
> >> > >
> >> > > 2. Even if this code is used for parsing, it would bring a lot of
> >> > > confusion. For now, we can say that docs/README.video does not apply to
> >> > > H3 and newer SoCs. If we implement this only partially, we would need to
> >> > > describe in details which of each setting is honored with the new driver
> >> > > and which not. Even then, a lot of users would skip that description and
> >> > > complain anyway.
> >> > The issue with this, and we've been bitten very hard on this one with
> >> > the CHIP, is that you don't really have a clear majority on that
> >> > one. If you support only PAL, half the world will be left out, and
> >> > same thing with NTSC (for some reason, we never needed to support the
> >> > less common ones like PAL-M or NTSC-J, but that just might be because
> >> > it never really sold that well in those countries, I don't have any
> >> > numbers on that).
> >> >
> >> > The point is, if you just hardcode PAL for now, you will have half
> >> > your users complain, and then, when we will introduce NTSC support
> >> > eventually, we'll have to introduce some mechanism to switch between
> >> > the two, then we'll probably break the behaviour our users relied on
> >> > before, making the other half of our users pissed.
> >> >
> >> > I'm not sure this is something we should just discard, or at least the
> >> > second part. Having the selection mechanism in place, even if we don't
> >> > support all the settings and just report an error in the logs in such
> >> > a case address the latter issue.
> >> >
> >> > You'll also need to address how to setup the overscan, since this is
> >> > really something you want to have very quick.
> >>
> >> Ok, I'm prepared to tackle this. Do you think it is worth to delay driver
> >> merge?
> >
> > Not per se, but that should definitely be part of the same release, so
> > if you can make it by then, then we can merge that right now, and
> > merge the rest later. If you can't, then we'll have to postpone it a
> > bit.
> >
> >> > > 3. If anything is done in this direction, I think that it is better
> >> > > to extend DM video framework so other drivers would benefit from
> >> > > that work too.
> >> >
> >> > That makes sense, but again, this is a pre-requisite for me. And it's
> >> > not that hard to support the video modelines with a device model
> >> > driver, Linux does it, and you have a string identifier at the
> >> > beginning of it. It just has to be deterministic, but I don't think
> >> > this is an issue with U-Boot's DM.
> >>
> >> Ok, so how do you think we should implement this? If only composite
> >> modes are supported in the string, someone might try to set hdmi
> >> monitor. Should we just ignore such settings and maybe print a
> >> warning?
> >
> > I'm not sure it was your question, but I would just reject any
> > improper configuration, and not display anything in such a case.
> >
> >> Also the question for Simon, how should I merge code for parsing
> >> video string from drivers/video/videomodes.c to DM video in a way to
> >> be useful to most drivers?
> >>
> >> I guess this is the first time to support analog video output in DM
> >> video driver and there are some things missing such as a way to
> >> select TV standard and define overscan.
> >
> > I think it can be done in the same way linux does it here:
> > http://elixir.free-electrons.com/linux/latest/source/drivers/video/fbdev/core/fb_cmdline.c#L35
> >
> > You basically check first that there is a string matching what your
> > driver expects, and then get the options.
> >
> > We could imagine having some extra function to parse that string and
> > give back a structure filled with whatever was set in that command
> > line. We could imagine having the common custom properties to be
> > parsed at that same time.
> >
> >> Or alternatively, I could make just quick edit to sunxi_tve.c driver
> >> and directly use old functions as they are. That way we could get
> >> something working very quickly, but I don't like much such approach.
> >
> > That's another approach, I'm fine with it as a temporary measure, but
> > I'm not sure how Simon feels about it.
> 
> Well how about a function that decodes the string into a struct? That
> function could be called by any drivers that need it.

That was my first suggestion, so I'm fine with that :)

Maxime
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-sunxi/tve.h b/arch/arm/include/asm/arch-sunxi/tve.h
index 41a14a68e4..ff34bbbc12 100644
--- a/arch/arm/include/asm/arch-sunxi/tve.h
+++ b/arch/arm/include/asm/arch-sunxi/tve.h
@@ -45,7 +45,9 @@  struct sunxi_tve_reg {
 	u32 csc_reg1;			/* 0x044 */
 	u32 csc_reg2;			/* 0x048 */
 	u32 csc_reg3;			/* 0x04c */
-	u8 res1[0xb0];			/* 0x050 */
+	u8 res1[0xa8];			/* 0x050 */
+	u32 auto_detect_cfg0;		/* 0x0f8 */
+	u32 auto_detect_cfg1;		/* 0x0fc */
 	u32 color_burst;		/* 0x100 */
 	u32 vsync_num;			/* 0x104 */
 	u32 notch_freq;			/* 0x108 */
@@ -62,6 +64,10 @@  struct sunxi_tve_reg {
 	u32 slave_para;			/* 0x134 */
 	u32 cfg1;			/* 0x138 */
 	u32 cfg2;			/* 0x13c */
+	u8 res2[0x1c4];			/* 0x140 */
+	u32 calibration;		/* 0x304 */
+	u8 res3[0x4];			/* 0x308 */
+	u32 unknown3;			/* 0x30c */
 };
 
 /*
@@ -79,12 +85,14 @@  struct sunxi_tve_reg {
 #define SUNXI_TVE_CFG0_PAL			0x07030001
 #define SUNXI_TVE_CFG0_NTSC			0x07030000
 #define SUNXI_TVE_DAC_CFG0_VGA			0x403e1ac7
-#ifdef CONFIG_MACH_SUN5I
+#if defined(CONFIG_MACH_SUN5I) || defined(CONFIG_MACH_SUNXI_H3_H5)
 #define SUNXI_TVE_DAC_CFG0_COMPOSITE		0x433f0009
 #else
 #define SUNXI_TVE_DAC_CFG0_COMPOSITE		0x403f0008
 #endif
+#define SUNXI_TVE_DAC_CFG0_DETECTION		0x433f0289
 #define SUNXI_TVE_FILTER_COMPOSITE		0x00000120
+#define SUNXI_TVE_CHROMA_FREQ_PAL		0x2a098acb
 #define SUNXI_TVE_CHROMA_FREQ_PAL_M		0x21e6efe3
 #define SUNXI_TVE_CHROMA_FREQ_PAL_NC		0x21f69446
 #define SUNXI_TVE_PORCH_NUM_PAL			0x008a0018
@@ -105,6 +113,8 @@  struct sunxi_tve_reg {
 #define SUNXI_TVE_AUTO_DETECT_STATUS_SHORT_GND	3
 #define SUNXI_TVE_AUTO_DETECT_DEBOUNCE_SHIFT(d)	((d) * 8)
 #define SUNXI_TVE_AUTO_DETECT_DEBOUNCE_MASK(d)	(0xf << ((d) * 8))
+#define SUNXI_TVE_AUTO_DETECT_CFG0		0x00000280
+#define SUNXI_TVE_AUTO_DETECT_CFG1		0x028F00FF
 #define SUNXI_TVE_CSC_REG0_ENABLE		(1 << 31)
 #define SUNXI_TVE_CSC_REG0			0x08440832
 #define SUNXI_TVE_CSC_REG1			0x3b6dace1
@@ -124,6 +134,9 @@  struct sunxi_tve_reg {
 #define SUNXI_TVE_RESYNC_NUM_PAL		0x800d000c
 #define SUNXI_TVE_RESYNC_NUM_NTSC		0x000e000c
 #define SUNXI_TVE_SLAVE_PARA_COMPOSITE		0x00000000
+#define SUNXI_TVE_CALIBRATION_H3		0x02000c00
+#define SUNXI_TVE_CALIBRATION_H5		0x02850000
+#define SUNXI_TVE_UNKNOWN3_H5			0x00101110
 
 void tvencoder_mode_set(struct sunxi_tve_reg * const tve, enum tve_mode mode);
 void tvencoder_enable(struct sunxi_tve_reg * const tve);
diff --git a/drivers/video/sunxi/Makefile b/drivers/video/sunxi/Makefile
index 0d64c2021f..e63c1d65bc 100644
--- a/drivers/video/sunxi/Makefile
+++ b/drivers/video/sunxi/Makefile
@@ -6,4 +6,4 @@ 
 #
 
 obj-$(CONFIG_VIDEO_SUNXI) += sunxi_display.o lcdc.o tve_common.o ../videomodes.o
-obj-$(CONFIG_VIDEO_DE2) += sunxi_de2.o sunxi_dw_hdmi.o lcdc.o ../dw_hdmi.o
+obj-$(CONFIG_VIDEO_DE2) += sunxi_de2.o sunxi_dw_hdmi.o sunxi_tve.o lcdc.o tve_common.o ../dw_hdmi.o
diff --git a/drivers/video/sunxi/sunxi_tve.c b/drivers/video/sunxi/sunxi_tve.c
new file mode 100644
index 0000000000..95f54bbaf7
--- /dev/null
+++ b/drivers/video/sunxi/sunxi_tve.c
@@ -0,0 +1,156 @@ 
+/*
+ * Allwinner TVE driver
+ *
+ * (C) Copyright 2017 Jernej Skrabec <jernej.skrabec@siol.net>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <display.h>
+#include <dm.h>
+#include <asm/io.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/lcdc.h>
+#include <asm/arch/tve.h>
+
+static int sunxi_tve_get_plug_in_status(void)
+{
+	struct sunxi_tve_reg * const tve =
+		(struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
+	u32 status;
+
+	status = readl(&tve->auto_detect_status) &
+		SUNXI_TVE_AUTO_DETECT_STATUS_MASK(0);
+
+	return status == SUNXI_TVE_AUTO_DETECT_STATUS_CONNECTED;
+}
+
+static int sunxi_tve_wait_for_hpd(void)
+{
+	struct sunxi_tve_reg * const tve =
+		(struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
+	ulong start;
+
+	/* enable auto detection */
+	writel(SUNXI_TVE_DAC_CFG0_DETECTION, &tve->dac_cfg0);
+	writel(SUNXI_TVE_AUTO_DETECT_CFG0, &tve->auto_detect_cfg0);
+	writel(SUNXI_TVE_AUTO_DETECT_CFG1, &tve->auto_detect_cfg1);
+	writel(9 << SUNXI_TVE_AUTO_DETECT_DEBOUNCE_SHIFT(0),
+	       &tve->auto_detect_debounce);
+	writel(SUNXI_TVE_AUTO_DETECT_EN_DET_EN(0), &tve->auto_detect_en);
+
+	start = get_timer(0);
+	do {
+		if (sunxi_tve_get_plug_in_status())
+			return 0;
+		udelay(100);
+	} while (get_timer(start) < 300);
+
+	return -1;
+}
+
+static void sunxi_tve_lcdc_init(const struct display_timing *edid, int bpp)
+{
+	struct sunxi_ccm_reg * const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+	struct sunxi_lcdc_reg * const lcdc =
+		(struct sunxi_lcdc_reg *)SUNXI_LCD1_BASE;
+
+	/* Reset off */
+	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_LCD1);
+
+	/* Clock on */
+	setbits_le32(&ccm->ahb_gate1, 1 << AHB_GATE_OFFSET_LCD1);
+
+	lcdc_init(lcdc);
+	lcdc_tcon1_mode_set(lcdc, edid, false, true);
+	lcdc_enable(lcdc, bpp);
+}
+
+static int sunxi_tve_read_timing(struct udevice *dev,
+				 struct display_timing *timing)
+{
+	/* PAL resolution */
+	timing->pixelclock.typ = 27000000;
+
+	timing->hactive.typ = 720;
+	timing->hfront_porch.typ = 5;
+	timing->hback_porch.typ = 137;
+	timing->hsync_len.typ = 2;
+
+	timing->vactive.typ = 576;
+	timing->vfront_porch.typ = 27;
+	timing->vback_porch.typ = 20;
+	timing->vsync_len.typ = 2;
+
+	timing->flags = DISPLAY_FLAGS_INTERLACED;
+
+	return 0;
+}
+
+static int sunxi_tve_enable(struct udevice *dev, int panel_bpp,
+			    const struct display_timing *edid)
+{
+	struct sunxi_tve_reg * const tve =
+		(struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
+
+	sunxi_tve_lcdc_init(edid, panel_bpp);
+
+	tvencoder_mode_set(tve, tve_mode_composite_pal);
+	tvencoder_enable(tve);
+
+	return 0;
+}
+
+static int sunxi_tve_probe(struct udevice *dev)
+{
+	struct sunxi_ccm_reg * const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+	struct sunxi_tve_reg * const tve =
+		(struct sunxi_tve_reg *)SUNXI_TVE0_BASE;
+	int ret;
+
+	/* make sure that clock is active */
+	clock_set_pll10(432000000);
+
+	/* Reset off */
+	setbits_le32(&ccm->ahb_reset1_cfg, 1 << AHB_RESET_OFFSET_TVE);
+
+	/* Clock on */
+	setbits_le32(&ccm->ahb_gate1, 1 << AHB_GATE_OFFSET_TVE);
+	writel(CCM_TVE_CTRL_GATE | CCM_TVE_CTRL_M(2), &ccm->tve_clk_cfg);
+
+#ifdef CONFIG_MACH_SUN50I_H5
+	writel(SUNXI_TVE_CALIBRATION_H5, &tve->calibration);
+	writel(SUNXI_TVE_UNKNOWN3_H5, &tve->unknown3);
+#else
+	writel(SUNXI_TVE_CALIBRATION_H3, &tve->calibration);
+#endif
+
+	ret = sunxi_tve_wait_for_hpd();
+	if (ret < 0) {
+		debug("tve can not get hpd signal\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+static const struct dm_display_ops sunxi_tve_ops = {
+	.read_timing = sunxi_tve_read_timing,
+	.enable = sunxi_tve_enable,
+};
+
+U_BOOT_DRIVER(sunxi_tve) = {
+	.name	= "sunxi_tve",
+	.id	= UCLASS_DISPLAY,
+	.ops	= &sunxi_tve_ops,
+	.probe	= sunxi_tve_probe,
+};
+
+#ifdef CONFIG_MACH_SUNXI_H3_H5
+U_BOOT_DEVICE(sunxi_tve) = {
+	.name = "sunxi_tve"
+};
+#endif
diff --git a/drivers/video/sunxi/tve_common.c b/drivers/video/sunxi/tve_common.c
index adea78a69a..ef99c111e3 100644
--- a/drivers/video/sunxi/tve_common.c
+++ b/drivers/video/sunxi/tve_common.c
@@ -25,8 +25,6 @@  void tvencoder_mode_set(struct sunxi_tve_reg * const tve, enum tve_mode mode)
 		writel(SUNXI_TVE_UNKNOWN1_VGA, &tve->unknown1);
 		break;
 	case tve_mode_composite_pal_nc:
-		writel(SUNXI_TVE_CHROMA_FREQ_PAL_NC, &tve->chroma_freq);
-		/* Fall through */
 	case tve_mode_composite_pal:
 		writel(SUNXI_TVE_GCTRL_DAC_INPUT(0, 1) |
 		       SUNXI_TVE_GCTRL_DAC_INPUT(1, 2) |
@@ -35,6 +33,10 @@  void tvencoder_mode_set(struct sunxi_tve_reg * const tve, enum tve_mode mode)
 		writel(SUNXI_TVE_CFG0_PAL, &tve->cfg0);
 		writel(SUNXI_TVE_DAC_CFG0_COMPOSITE, &tve->dac_cfg0);
 		writel(SUNXI_TVE_FILTER_COMPOSITE, &tve->filter);
+		if (mode == tve_mode_composite_pal)
+			writel(SUNXI_TVE_CHROMA_FREQ_PAL, &tve->chroma_freq);
+		else
+			writel(SUNXI_TVE_CHROMA_FREQ_PAL_NC, &tve->chroma_freq);
 		writel(SUNXI_TVE_PORCH_NUM_PAL, &tve->porch_num);
 		writel(SUNXI_TVE_LINE_NUM_PAL, &tve->line_num);
 		writel(SUNXI_TVE_BLANK_BLACK_LEVEL_PAL,