diff mbox series

[v5,22/22] drm/sun4i: tv: Convert to the new TV mode property

Message ID 20220728-rpi-analog-tv-properties-v5-22-d841cc64fe4b@cerno.tech (mailing list archive)
State New, archived
Headers show
Series drm: Analog TV Improvements | expand

Commit Message

Maxime Ripard Oct. 13, 2022, 1:19 p.m. UTC
Now that the core can deal fine with analog TV modes, let's convert the
sun4i TV driver to leverage those new features.

Acked-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Maxime Ripard <maxime@cerno.tech>

---
Changes in v5:
- Removed the count variable in get_modes
- Removed spurious vc4 change
---
 drivers/gpu/drm/sun4i/sun4i_tv.c | 145 +++++++++++++--------------------------
 1 file changed, 48 insertions(+), 97 deletions(-)

Comments

Jernej Škrabec Oct. 13, 2022, 6:23 p.m. UTC | #1
Hi Maxime,

Dne četrtek, 13. oktober 2022 ob 15:19:06 CEST je Maxime Ripard napisal(a):
> Now that the core can deal fine with analog TV modes, let's convert the
> sun4i TV driver to leverage those new features.
> 
> Acked-by: Noralf Trønnes <noralf@tronnes.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> 
> ---
> Changes in v5:
> - Removed the count variable in get_modes
> - Removed spurious vc4 change
> ---
>  drivers/gpu/drm/sun4i/sun4i_tv.c | 145
> +++++++++++++-------------------------- 1 file changed, 48 insertions(+),
> 97 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
> b/drivers/gpu/drm/sun4i/sun4i_tv.c index c65f0a89b6b0..4f07aff11551 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> @@ -141,23 +141,14 @@ struct resync_parameters {
>  struct tv_mode {
>  	char		*name;
> 
> +	unsigned int	tv_mode;
> +
>  	u32		mode;
>  	u32		chroma_freq;
>  	u16		back_porch;
>  	u16		front_porch;
> -	u16		line_number;
>  	u16		vblank_level;

isn't there a way to get or calculate back_porch, front_porch and vblank_level 
from mode? From quick glance over removed values below, I would say that at 
least back_porch can be removed too?

Otherwise this patch looks ok.

Best regards,
Jernej

> 
> -	u32		hdisplay;
> -	u16		hfront_porch;
> -	u16		hsync_len;
> -	u16		hback_porch;
> -
> -	u32		vdisplay;
> -	u16		vfront_porch;
> -	u16		vsync_len;
> -	u16		vback_porch;
> -
>  	bool		yc_en;
>  	bool		dac3_en;
>  	bool		dac_bit25_en;
> @@ -213,7 +204,7 @@ static const struct resync_parameters
> pal_resync_parameters = {
> 
>  static const struct tv_mode tv_modes[] = {
>  	{
> -		.name		= "NTSC",
> +		.tv_mode	= DRM_MODE_TV_MODE_NTSC,
>  		.mode		= SUN4I_TVE_CFG0_RES_480i,
>  		.chroma_freq	= 0x21f07c1f,
>  		.yc_en		= true,
> @@ -222,17 +213,6 @@ static const struct tv_mode tv_modes[] = {
> 
>  		.back_porch	= 118,
>  		.front_porch	= 32,
> -		.line_number	= 525,
> -
> -		.hdisplay	= 720,
> -		.hfront_porch	= 18,
> -		.hsync_len	= 2,
> -		.hback_porch	= 118,
> -
> -		.vdisplay	= 480,
> -		.vfront_porch	= 26,
> -		.vsync_len	= 2,
> -		.vback_porch	= 17,
> 
>  		.vblank_level	= 240,
> 
> @@ -242,23 +222,12 @@ static const struct tv_mode tv_modes[] = {
>  		.resync_params	= &ntsc_resync_parameters,
>  	},
>  	{
> -		.name		= "PAL",
> +		.tv_mode	= DRM_MODE_TV_MODE_PAL,
>  		.mode		= SUN4I_TVE_CFG0_RES_576i,
>  		.chroma_freq	= 0x2a098acb,
> 
>  		.back_porch	= 138,
>  		.front_porch	= 24,
> -		.line_number	= 625,
> -
> -		.hdisplay	= 720,
> -		.hfront_porch	= 3,
> -		.hsync_len	= 2,
> -		.hback_porch	= 139,
> -
> -		.vdisplay	= 576,
> -		.vfront_porch	= 28,
> -		.vsync_len	= 2,
> -		.vback_porch	= 19,
> 
>  		.vblank_level	= 252,
> 
> @@ -276,63 +245,21 @@ drm_encoder_to_sun4i_tv(struct drm_encoder *encoder)
>  			    encoder);
>  }
> 
> -/*
> - * FIXME: If only the drm_display_mode private field was usable, this
> - * could go away...
> - *
> - * So far, it doesn't seem to be preserved when the mode is passed by
> - * to mode_set for some reason.
> - */
> -static const struct tv_mode *sun4i_tv_find_tv_by_mode(const struct
> drm_display_mode *mode) +static const struct tv_mode *
> +sun4i_tv_find_tv_by_mode(unsigned int mode)
>  {
>  	int i;
> 
> -	/* First try to identify the mode by name */
>  	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
>  		const struct tv_mode *tv_mode = &tv_modes[i];
> 
> -		DRM_DEBUG_DRIVER("Comparing mode %s vs %s",
> -				 mode->name, tv_mode->name);
> -
> -		if (!strcmp(mode->name, tv_mode->name))
> -			return tv_mode;
> -	}
> -
> -	/* Then by number of lines */
> -	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
> -		const struct tv_mode *tv_mode = &tv_modes[i];
> -
> -		DRM_DEBUG_DRIVER("Comparing mode %s vs %s (X: %d vs 
%d)",
> -				 mode->name, tv_mode->name,
> -				 mode->vdisplay, tv_mode-
>vdisplay);
> -
> -		if (mode->vdisplay == tv_mode->vdisplay)
> +		if (tv_mode->tv_mode == mode)
>  			return tv_mode;
>  	}
> 
>  	return NULL;
>  }
> 
> -static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
> -				      struct drm_display_mode 
*mode)
> -{
> -	DRM_DEBUG_DRIVER("Creating mode %s\n", mode->name);
> -
> -	mode->type = DRM_MODE_TYPE_DRIVER;
> -	mode->clock = 13500;
> -	mode->flags = DRM_MODE_FLAG_INTERLACE;
> -
> -	mode->hdisplay = tv_mode->hdisplay;
> -	mode->hsync_start = mode->hdisplay + tv_mode->hfront_porch;
> -	mode->hsync_end = mode->hsync_start + tv_mode->hsync_len;
> -	mode->htotal = mode->hsync_end  + tv_mode->hback_porch;
> -
> -	mode->vdisplay = tv_mode->vdisplay;
> -	mode->vsync_start = mode->vdisplay + tv_mode->vfront_porch;
> -	mode->vsync_end = mode->vsync_start + tv_mode->vsync_len;
> -	mode->vtotal = mode->vsync_end  + tv_mode->vback_porch;
> -}
> -
>  static void sun4i_tv_disable(struct drm_encoder *encoder,
>  			    struct drm_atomic_state *state)
>  {
> @@ -356,7 +283,11 @@ static void sun4i_tv_enable(struct drm_encoder
> *encoder, struct drm_crtc_state *crtc_state =
>  		drm_atomic_get_new_crtc_state(state, encoder->crtc);
>  	struct drm_display_mode *mode = &crtc_state->mode;
> -	const struct tv_mode *tv_mode = sun4i_tv_find_tv_by_mode(mode);
> +	struct drm_connector *connector = &tv->connector;
> +	struct drm_connector_state *conn_state =
> +		drm_atomic_get_new_connector_state(state, connector);
> +	const struct tv_mode *tv_mode =
> +		sun4i_tv_find_tv_by_mode(conn_state->tv.mode);
> 
>  	DRM_DEBUG_DRIVER("Enabling the TV Output\n");
> 
> @@ -404,7 +335,7 @@ static void sun4i_tv_enable(struct drm_encoder *encoder,
> /* Set the lines setup */
>  	regmap_write(tv->regs, SUN4I_TVE_LINE_REG,
>  		     SUN4I_TVE_LINE_FIRST(22) |
> -		     SUN4I_TVE_LINE_NUMBER(tv_mode->line_number));
> +		     SUN4I_TVE_LINE_NUMBER(mode->vtotal));
> 
>  	regmap_write(tv->regs, SUN4I_TVE_LEVEL_REG,
>  		     SUN4I_TVE_LEVEL_BLANK(tv_mode->video_levels-
>blank) |
> @@ -467,35 +398,43 @@ static const struct drm_encoder_helper_funcs
> sun4i_tv_helper_funcs = {
> 
>  static int sun4i_tv_comp_get_modes(struct drm_connector *connector)
>  {
> -	int i;
> +	struct drm_display_mode *mode;
> 
> -	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
> -		struct drm_display_mode *mode;
> -		const struct tv_mode *tv_mode = &tv_modes[i];
> -
> -		mode = drm_mode_create(connector->dev);
> -		if (!mode) {
> -			DRM_ERROR("Failed to create a new display 
mode\n");
> -			return 0;
> -		}
> +	mode = drm_mode_analog_ntsc_480i(connector->dev);
> +	if (!mode) {
> +		DRM_ERROR("Failed to create a new display mode\n");
> +		return -ENOMEM;
> +	}
> 
> -		strcpy(mode->name, tv_mode->name);
> +	mode->type |= DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_probed_add(connector, mode);
> 
> -		sun4i_tv_mode_to_drm_mode(tv_mode, mode);
> -		drm_mode_probed_add(connector, mode);
> +	mode = drm_mode_analog_pal_576i(connector->dev);
> +	if (!mode) {
> +		DRM_ERROR("Failed to create a new display mode\n");
> +		return -ENOMEM;
>  	}
> 
> -	return i;
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 2;
>  }
> 
>  static const struct drm_connector_helper_funcs
> sun4i_tv_comp_connector_helper_funcs = { +	.atomic_check	=
> drm_atomic_helper_connector_tv_check,
>  	.get_modes	= sun4i_tv_comp_get_modes,
>  };
> 
> +static void sun4i_tv_connector_reset(struct drm_connector *connector)
> +{
> +	drm_atomic_helper_connector_reset(connector);
> +	drm_atomic_helper_connector_tv_reset(connector);
> +}
> +
>  static const struct drm_connector_funcs sun4i_tv_comp_connector_funcs = {
>  	.fill_modes		= drm_helper_probe_single_connector_modes,
>  	.destroy		= drm_connector_cleanup,
> -	.reset			= 
drm_atomic_helper_connector_reset,
> +	.reset			= sun4i_tv_connector_reset,
>  	.atomic_duplicate_state	= 
drm_atomic_helper_connector_duplicate_state,
>  	.atomic_destroy_state	= 
drm_atomic_helper_connector_destroy_state,
>  };
> @@ -587,8 +526,20 @@ static int sun4i_tv_bind(struct device *dev, struct
> device *master,
> 
>  	drm_connector_attach_encoder(&tv->connector, &tv->encoder);
> 
> +	ret = drm_mode_create_tv_properties(drm,
> +					    
BIT(DRM_MODE_TV_MODE_NTSC) |
> +					    
BIT(DRM_MODE_TV_MODE_PAL));
> +	if (ret)
> +		goto err_cleanup_connector;
> +
> +	drm_object_attach_property(&tv->connector.base,
> +				   drm-
>mode_config.tv_mode_property,
> +				   DRM_MODE_TV_MODE_NTSC);
> +
>  	return 0;
> 
> +err_cleanup_connector:
> +	drm_connector_cleanup(&tv->connector);
>  err_cleanup_encoder:
>  	drm_encoder_cleanup(&tv->encoder);
>  err_disable_clk:
> 
> --
> b4 0.11.0-dev-7da52
Maxime Ripard Oct. 14, 2022, 7:38 a.m. UTC | #2
Hi Jernej,

On Thu, Oct 13, 2022 at 08:23:51PM +0200, Jernej Škrabec wrote:
> Dne četrtek, 13. oktober 2022 ob 15:19:06 CEST je Maxime Ripard napisal(a):
> > Now that the core can deal fine with analog TV modes, let's convert the
> > sun4i TV driver to leverage those new features.
> > 
> > Acked-by: Noralf Trønnes <noralf@tronnes.org>
> > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > 
> > ---
> > Changes in v5:
> > - Removed the count variable in get_modes
> > - Removed spurious vc4 change
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_tv.c | 145
> > +++++++++++++-------------------------- 1 file changed, 48 insertions(+),
> > 97 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > b/drivers/gpu/drm/sun4i/sun4i_tv.c index c65f0a89b6b0..4f07aff11551 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> > @@ -141,23 +141,14 @@ struct resync_parameters {
> >  struct tv_mode {
> >  	char		*name;
> > 
> > +	unsigned int	tv_mode;
> > +
> >  	u32		mode;
> >  	u32		chroma_freq;
> >  	u16		back_porch;
> >  	u16		front_porch;
> > -	u16		line_number;
> >  	u16		vblank_level;
> 
> isn't there a way to get or calculate back_porch, front_porch and vblank_level 
> from mode? From quick glance over removed values below, I would say that at 
> least back_porch can be removed too?

I tried actually, but I'm not sure what the front porch and back porch
parameters actually are. They are called that way by Allwinner, but it
doesn't match the PAL or NTSC timings at all.

For example, back_porch is 118 for NTSC and 138 for PAL. Actual back
porches would be around 12 and 16, respectively. Actually, the entire
blanking area are 138 and 144. This is close enough for PAL, but pretty
far off for NTSC.

Allwinner has the habit of integrating the sync period into one of the
porches, but still there's no obvious match.

front_porch is pretty much in the same case.

Since it affected the display output quite a lot, I chose to keep it for
now unfortunately.

> Otherwise this patch looks ok.

Can I add your Acked-by/Reviewed-by then?

Thanks!
Maxime
Jernej Škrabec Oct. 15, 2022, 8:59 a.m. UTC | #3
Dne petek, 14. oktober 2022 ob 09:38:10 CEST je Maxime Ripard napisal(a):
> Hi Jernej,
> 
> On Thu, Oct 13, 2022 at 08:23:51PM +0200, Jernej Škrabec wrote:
> > Dne četrtek, 13. oktober 2022 ob 15:19:06 CEST je Maxime Ripard 
napisal(a):
> > > Now that the core can deal fine with analog TV modes, let's convert the
> > > sun4i TV driver to leverage those new features.
> > > 
> > > Acked-by: Noralf Trønnes <noralf@tronnes.org>
> > > Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> > > 
> > > ---
> > > Changes in v5:
> > > - Removed the count variable in get_modes
> > > - Removed spurious vc4 change
> > > ---
> > > 
> > >  drivers/gpu/drm/sun4i/sun4i_tv.c | 145
> > > 
> > > +++++++++++++-------------------------- 1 file changed, 48
> > > insertions(+),
> > > 97 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > > b/drivers/gpu/drm/sun4i/sun4i_tv.c index c65f0a89b6b0..4f07aff11551
> > > 100644
> > > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c
> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
> > > @@ -141,23 +141,14 @@ struct resync_parameters {
> > > 
> > >  struct tv_mode {
> > >  
> > >  	char		*name;
> > > 
> > > +	unsigned int	tv_mode;
> > > +
> > > 
> > >  	u32		mode;
> > >  	u32		chroma_freq;
> > >  	u16		back_porch;
> > >  	u16		front_porch;
> > > 
> > > -	u16		line_number;
> > > 
> > >  	u16		vblank_level;
> > 
> > isn't there a way to get or calculate back_porch, front_porch and
> > vblank_level from mode? From quick glance over removed values below, I
> > would say that at least back_porch can be removed too?
> 
> I tried actually, but I'm not sure what the front porch and back porch
> parameters actually are. They are called that way by Allwinner, but it
> doesn't match the PAL or NTSC timings at all.
> 
> For example, back_porch is 118 for NTSC and 138 for PAL. Actual back
> porches would be around 12 and 16, respectively. Actually, the entire
> blanking area are 138 and 144. This is close enough for PAL, but pretty
> far off for NTSC.
> 
> Allwinner has the habit of integrating the sync period into one of the
> porches, but still there's no obvious match.
> 
> front_porch is pretty much in the same case.

Ok then.

> 
> Since it affected the display output quite a lot, I chose to keep it for
> now unfortunately.
> 
> > Otherwise this patch looks ok.
> 
> Can I add your Acked-by/Reviewed-by then?

Sure.
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej
diff mbox series

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c b/drivers/gpu/drm/sun4i/sun4i_tv.c
index c65f0a89b6b0..4f07aff11551 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tv.c
@@ -141,23 +141,14 @@  struct resync_parameters {
 struct tv_mode {
 	char		*name;
 
+	unsigned int	tv_mode;
+
 	u32		mode;
 	u32		chroma_freq;
 	u16		back_porch;
 	u16		front_porch;
-	u16		line_number;
 	u16		vblank_level;
 
-	u32		hdisplay;
-	u16		hfront_porch;
-	u16		hsync_len;
-	u16		hback_porch;
-
-	u32		vdisplay;
-	u16		vfront_porch;
-	u16		vsync_len;
-	u16		vback_porch;
-
 	bool		yc_en;
 	bool		dac3_en;
 	bool		dac_bit25_en;
@@ -213,7 +204,7 @@  static const struct resync_parameters pal_resync_parameters = {
 
 static const struct tv_mode tv_modes[] = {
 	{
-		.name		= "NTSC",
+		.tv_mode	= DRM_MODE_TV_MODE_NTSC,
 		.mode		= SUN4I_TVE_CFG0_RES_480i,
 		.chroma_freq	= 0x21f07c1f,
 		.yc_en		= true,
@@ -222,17 +213,6 @@  static const struct tv_mode tv_modes[] = {
 
 		.back_porch	= 118,
 		.front_porch	= 32,
-		.line_number	= 525,
-
-		.hdisplay	= 720,
-		.hfront_porch	= 18,
-		.hsync_len	= 2,
-		.hback_porch	= 118,
-
-		.vdisplay	= 480,
-		.vfront_porch	= 26,
-		.vsync_len	= 2,
-		.vback_porch	= 17,
 
 		.vblank_level	= 240,
 
@@ -242,23 +222,12 @@  static const struct tv_mode tv_modes[] = {
 		.resync_params	= &ntsc_resync_parameters,
 	},
 	{
-		.name		= "PAL",
+		.tv_mode	= DRM_MODE_TV_MODE_PAL,
 		.mode		= SUN4I_TVE_CFG0_RES_576i,
 		.chroma_freq	= 0x2a098acb,
 
 		.back_porch	= 138,
 		.front_porch	= 24,
-		.line_number	= 625,
-
-		.hdisplay	= 720,
-		.hfront_porch	= 3,
-		.hsync_len	= 2,
-		.hback_porch	= 139,
-
-		.vdisplay	= 576,
-		.vfront_porch	= 28,
-		.vsync_len	= 2,
-		.vback_porch	= 19,
 
 		.vblank_level	= 252,
 
@@ -276,63 +245,21 @@  drm_encoder_to_sun4i_tv(struct drm_encoder *encoder)
 			    encoder);
 }
 
-/*
- * FIXME: If only the drm_display_mode private field was usable, this
- * could go away...
- *
- * So far, it doesn't seem to be preserved when the mode is passed by
- * to mode_set for some reason.
- */
-static const struct tv_mode *sun4i_tv_find_tv_by_mode(const struct drm_display_mode *mode)
+static const struct tv_mode *
+sun4i_tv_find_tv_by_mode(unsigned int mode)
 {
 	int i;
 
-	/* First try to identify the mode by name */
 	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
 		const struct tv_mode *tv_mode = &tv_modes[i];
 
-		DRM_DEBUG_DRIVER("Comparing mode %s vs %s",
-				 mode->name, tv_mode->name);
-
-		if (!strcmp(mode->name, tv_mode->name))
-			return tv_mode;
-	}
-
-	/* Then by number of lines */
-	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
-		const struct tv_mode *tv_mode = &tv_modes[i];
-
-		DRM_DEBUG_DRIVER("Comparing mode %s vs %s (X: %d vs %d)",
-				 mode->name, tv_mode->name,
-				 mode->vdisplay, tv_mode->vdisplay);
-
-		if (mode->vdisplay == tv_mode->vdisplay)
+		if (tv_mode->tv_mode == mode)
 			return tv_mode;
 	}
 
 	return NULL;
 }
 
-static void sun4i_tv_mode_to_drm_mode(const struct tv_mode *tv_mode,
-				      struct drm_display_mode *mode)
-{
-	DRM_DEBUG_DRIVER("Creating mode %s\n", mode->name);
-
-	mode->type = DRM_MODE_TYPE_DRIVER;
-	mode->clock = 13500;
-	mode->flags = DRM_MODE_FLAG_INTERLACE;
-
-	mode->hdisplay = tv_mode->hdisplay;
-	mode->hsync_start = mode->hdisplay + tv_mode->hfront_porch;
-	mode->hsync_end = mode->hsync_start + tv_mode->hsync_len;
-	mode->htotal = mode->hsync_end  + tv_mode->hback_porch;
-
-	mode->vdisplay = tv_mode->vdisplay;
-	mode->vsync_start = mode->vdisplay + tv_mode->vfront_porch;
-	mode->vsync_end = mode->vsync_start + tv_mode->vsync_len;
-	mode->vtotal = mode->vsync_end  + tv_mode->vback_porch;
-}
-
 static void sun4i_tv_disable(struct drm_encoder *encoder,
 			    struct drm_atomic_state *state)
 {
@@ -356,7 +283,11 @@  static void sun4i_tv_enable(struct drm_encoder *encoder,
 	struct drm_crtc_state *crtc_state =
 		drm_atomic_get_new_crtc_state(state, encoder->crtc);
 	struct drm_display_mode *mode = &crtc_state->mode;
-	const struct tv_mode *tv_mode = sun4i_tv_find_tv_by_mode(mode);
+	struct drm_connector *connector = &tv->connector;
+	struct drm_connector_state *conn_state =
+		drm_atomic_get_new_connector_state(state, connector);
+	const struct tv_mode *tv_mode =
+		sun4i_tv_find_tv_by_mode(conn_state->tv.mode);
 
 	DRM_DEBUG_DRIVER("Enabling the TV Output\n");
 
@@ -404,7 +335,7 @@  static void sun4i_tv_enable(struct drm_encoder *encoder,
 	/* Set the lines setup */
 	regmap_write(tv->regs, SUN4I_TVE_LINE_REG,
 		     SUN4I_TVE_LINE_FIRST(22) |
-		     SUN4I_TVE_LINE_NUMBER(tv_mode->line_number));
+		     SUN4I_TVE_LINE_NUMBER(mode->vtotal));
 
 	regmap_write(tv->regs, SUN4I_TVE_LEVEL_REG,
 		     SUN4I_TVE_LEVEL_BLANK(tv_mode->video_levels->blank) |
@@ -467,35 +398,43 @@  static const struct drm_encoder_helper_funcs sun4i_tv_helper_funcs = {
 
 static int sun4i_tv_comp_get_modes(struct drm_connector *connector)
 {
-	int i;
+	struct drm_display_mode *mode;
 
-	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
-		struct drm_display_mode *mode;
-		const struct tv_mode *tv_mode = &tv_modes[i];
-
-		mode = drm_mode_create(connector->dev);
-		if (!mode) {
-			DRM_ERROR("Failed to create a new display mode\n");
-			return 0;
-		}
+	mode = drm_mode_analog_ntsc_480i(connector->dev);
+	if (!mode) {
+		DRM_ERROR("Failed to create a new display mode\n");
+		return -ENOMEM;
+	}
 
-		strcpy(mode->name, tv_mode->name);
+	mode->type |= DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
 
-		sun4i_tv_mode_to_drm_mode(tv_mode, mode);
-		drm_mode_probed_add(connector, mode);
+	mode = drm_mode_analog_pal_576i(connector->dev);
+	if (!mode) {
+		DRM_ERROR("Failed to create a new display mode\n");
+		return -ENOMEM;
 	}
 
-	return i;
+	drm_mode_probed_add(connector, mode);
+
+	return 2;
 }
 
 static const struct drm_connector_helper_funcs sun4i_tv_comp_connector_helper_funcs = {
+	.atomic_check	= drm_atomic_helper_connector_tv_check,
 	.get_modes	= sun4i_tv_comp_get_modes,
 };
 
+static void sun4i_tv_connector_reset(struct drm_connector *connector)
+{
+	drm_atomic_helper_connector_reset(connector);
+	drm_atomic_helper_connector_tv_reset(connector);
+}
+
 static const struct drm_connector_funcs sun4i_tv_comp_connector_funcs = {
 	.fill_modes		= drm_helper_probe_single_connector_modes,
 	.destroy		= drm_connector_cleanup,
-	.reset			= drm_atomic_helper_connector_reset,
+	.reset			= sun4i_tv_connector_reset,
 	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
 };
@@ -587,8 +526,20 @@  static int sun4i_tv_bind(struct device *dev, struct device *master,
 
 	drm_connector_attach_encoder(&tv->connector, &tv->encoder);
 
+	ret = drm_mode_create_tv_properties(drm,
+					    BIT(DRM_MODE_TV_MODE_NTSC) |
+					    BIT(DRM_MODE_TV_MODE_PAL));
+	if (ret)
+		goto err_cleanup_connector;
+
+	drm_object_attach_property(&tv->connector.base,
+				   drm->mode_config.tv_mode_property,
+				   DRM_MODE_TV_MODE_NTSC);
+
 	return 0;
 
+err_cleanup_connector:
+	drm_connector_cleanup(&tv->connector);
 err_cleanup_encoder:
 	drm_encoder_cleanup(&tv->encoder);
 err_disable_clk: