[PATCH 0/6] drm/vc4: VEC (SDTV) output support

[PATCH 0/6] drm/vc4: VEC (SDTV) output support

[Eric Anholt]

Here's a series from Boris for enabling SDTV output on VC4.  There are
two other patches to the clk subsystem necessary to get it working,
which I'm in the process of reviewing.

I've tested it on composite input of a monitor I had laying around,
and it's basically working.  Booting with the firmware bringing up
SDTV, I get an image "fine" (to the extent that SDTV is ever fine).
If I boot with firmware bringing up HDMI and switch to VEC later, then
the image is gray, but the clock got set to 106Mhz instead of 108 so
this is probably not a DRM bug.

This series was entirely written by Boris, but I've got the permission
to send it out so it's being routed through me.

Boris Brezillon (6):
  drm/vc4: Fix ->clock_select setting for the VEC encoder
  drm: Add TV connector states to drm_connector_state
  drm/vc4: Add support for the VEC (Video Encoder) IP
  drm/vc4: Document VEC DT binding
  ARM: bcm/dt: Add VEC node in bcm283x.dtsi
  ARM: bcm/dt: Enable the VEC IP on all RaspberryPi boards

 .../devicetree/bindings/display/brcm,bcm-vc4.txt   |  14 +
 arch/arm/boot/dts/bcm2835-rpi.dtsi                 |   5 +
 arch/arm/boot/dts/bcm283x.dtsi                     |   8 +
 drivers/gpu/drm/drm_atomic.c                       |  50 ++
 drivers/gpu/drm/vc4/Makefile                       |   1 +
 drivers/gpu/drm/vc4/vc4_crtc.c                     |  38 +-
 drivers/gpu/drm/vc4/vc4_debugfs.c                  |   1 +
 drivers/gpu/drm/vc4/vc4_drv.c                      |   1 +
 drivers/gpu/drm/vc4/vc4_drv.h                      |   6 +
 drivers/gpu/drm/vc4/vc4_regs.h                     |   3 +-
 drivers/gpu/drm/vc4/vc4_vec.c                      | 657 +++++++++++++++++++++
 include/drm/drm_connector.h                        |  32 +
 12 files changed, 800 insertions(+), 16 deletions(-)
 create mode 100644 drivers/gpu/drm/vc4/vc4_vec.c

-- 
2.10.2

[PATCH 1/6] drm/vc4: Fix ->clock_select setting for the VEC encoder

[Eric Anholt]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

PV_CONTROL_CLK_SELECT_VEC is actually 2 and not 0. Fix the definition and
rework the vc4_set_crtc_possible_masks() to cover the full range of the
PV_CONTROL_CLK_SELECT field.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 38 +++++++++++++++++++++++---------------
 drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
 drivers/gpu/drm/vc4/vc4_regs.h |  3 ++-
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 7f08d681a74b..c317e9103f9b 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -83,8 +83,7 @@ struct vc4_crtc_data {
 	/* Which channel of the HVS this pixelvalve sources from. */
 	int hvs_channel;
 
-	enum vc4_encoder_type encoder0_type;
-	enum vc4_encoder_type encoder1_type;
+	enum vc4_encoder_type encoder_types[4];
 };
 
 #define CRTC_WRITE(offset, val) writel(val, vc4_crtc->regs + (offset))
@@ -859,20 +858,26 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = {
 
 static const struct vc4_crtc_data pv0_data = {
 	.hvs_channel = 0,
-	.encoder0_type = VC4_ENCODER_TYPE_DSI0,
-	.encoder1_type = VC4_ENCODER_TYPE_DPI,
+	.encoder_types = {
+		[PV_CONTROL_CLK_SELECT_DSI] = VC4_ENCODER_TYPE_DSI0,
+		[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_DPI,
+	},
 };
 
 static const struct vc4_crtc_data pv1_data = {
 	.hvs_channel = 2,
-	.encoder0_type = VC4_ENCODER_TYPE_DSI1,
-	.encoder1_type = VC4_ENCODER_TYPE_SMI,
+	.encoder_types = {
+		[PV_CONTROL_CLK_SELECT_DSI] = VC4_ENCODER_TYPE_DSI1,
+		[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_SMI,
+	},
 };
 
 static const struct vc4_crtc_data pv2_data = {
 	.hvs_channel = 1,
-	.encoder0_type = VC4_ENCODER_TYPE_VEC,
-	.encoder1_type = VC4_ENCODER_TYPE_HDMI,
+	.encoder_types = {
+		[PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI] = VC4_ENCODER_TYPE_HDMI,
+		[PV_CONTROL_CLK_SELECT_VEC] = VC4_ENCODER_TYPE_VEC,
+	},
 };
 
 static const struct of_device_id vc4_crtc_dt_match[] = {
@@ -886,17 +891,20 @@ static void vc4_set_crtc_possible_masks(struct drm_device *drm,
 					struct drm_crtc *crtc)
 {
 	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
+	const struct vc4_crtc_data *crtc_data = vc4_crtc->data;
+	const enum vc4_encoder_type *encoder_types = crtc_data->encoder_types;
 	struct drm_encoder *encoder;
 
 	drm_for_each_encoder(encoder, drm) {
 		struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
-
-		if (vc4_encoder->type == vc4_crtc->data->encoder0_type) {
-			vc4_encoder->clock_select = 0;
-			encoder->possible_crtcs |= drm_crtc_mask(crtc);
-		} else if (vc4_encoder->type == vc4_crtc->data->encoder1_type) {
-			vc4_encoder->clock_select = 1;
-			encoder->possible_crtcs |= drm_crtc_mask(crtc);
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(crtc_data->encoder_types); i++) {
+			if (vc4_encoder->type == encoder_types[i]) {
+				vc4_encoder->clock_select = i;
+				encoder->possible_crtcs |= drm_crtc_mask(crtc);
+				break;
+			}
 		}
 	}
 }
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index fef172804345..b6049cc25675 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -194,6 +194,7 @@ to_vc4_plane(struct drm_plane *plane)
 }
 
 enum vc4_encoder_type {
+	VC4_ENCODER_TYPE_NONE,
 	VC4_ENCODER_TYPE_HDMI,
 	VC4_ENCODER_TYPE_VEC,
 	VC4_ENCODER_TYPE_DSI0,
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index 1aa44c2db556..39f6886b2410 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -177,8 +177,9 @@
 # define PV_CONTROL_WAIT_HSTART			BIT(12)
 # define PV_CONTROL_PIXEL_REP_MASK		VC4_MASK(5, 4)
 # define PV_CONTROL_PIXEL_REP_SHIFT		4
-# define PV_CONTROL_CLK_SELECT_DSI_VEC		0
+# define PV_CONTROL_CLK_SELECT_DSI		0
 # define PV_CONTROL_CLK_SELECT_DPI_SMI_HDMI	1
+# define PV_CONTROL_CLK_SELECT_VEC		2
 # define PV_CONTROL_CLK_SELECT_MASK		VC4_MASK(3, 2)
 # define PV_CONTROL_CLK_SELECT_SHIFT		2
 # define PV_CONTROL_FIFO_CLR			BIT(1)
-- 
2.10.2

[PATCH 2/6] drm: Add TV connector states to drm_connector_state

[Eric Anholt]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

Some generic TV connector properties are exposed in drm_mode_config, but
they are currently handled independently in each DRM encoder driver.

Extend the drm_connector_state to store TV related states, and modify the
drm_atomic_connector_{set,get}_property() helpers to fill the connector
state accordingly.

Each driver is then responsible for checking and applying the new config
in its ->atomic_mode_{check,set}() operations.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/drm_atomic.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h  | 32 ++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 23739609427d..02b0668f51e1 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -986,12 +986,38 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
 		 * now?) atomic writes to DPMS property:
 		 */
 		return -EINVAL;
+	} else if (property == config->tv_select_subconnector_property) {
+		state->tv.subconnector = val;
+	} else if (property == config->tv_left_margin_property) {
+		state->tv.margins.left = val;
+	} else if (property == config->tv_right_margin_property) {
+		state->tv.margins.right = val;
+	} else if (property == config->tv_top_margin_property) {
+		state->tv.margins.bottom = val;
+	} else if (property == config->tv_bottom_margin_property) {
+		state->tv.margins.right = val;
+	} else if (property == config->tv_mode_property) {
+		state->tv.mode = val;
+	} else if (property == config->tv_brightness_property) {
+		state->tv.brightness = val;
+	} else if (property == config->tv_contrast_property) {
+		state->tv.contrast = val;
+	} else if (property == config->tv_flicker_reduction_property) {
+		state->tv.flicker_reduction = val;
+	} else if (property == config->tv_overscan_property) {
+		state->tv.overscan = val;
+	} else if (property == config->tv_saturation_property) {
+		state->tv.saturation = val;
+	} else if (property == config->tv_hue_property) {
+		state->tv.hue = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
 	} else {
 		return -EINVAL;
 	}
+
+	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_connector_set_property);
 
@@ -1022,6 +1048,30 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = (state->crtc) ? state->crtc->base.id : 0;
 	} else if (property == config->dpms_property) {
 		*val = connector->dpms;
+	} else if (property == config->tv_select_subconnector_property) {
+		*val = state->tv.subconnector;
+	} else if (property == config->tv_left_margin_property) {
+		*val = state->tv.margins.left;
+	} else if (property == config->tv_right_margin_property) {
+		*val = state->tv.margins.right;
+	} else if (property == config->tv_top_margin_property) {
+		*val = state->tv.margins.bottom;
+	} else if (property == config->tv_bottom_margin_property) {
+		*val = state->tv.margins.right;
+	} else if (property == config->tv_mode_property) {
+		*val = state->tv.mode;
+	} else if (property == config->tv_brightness_property) {
+		*val = state->tv.brightness;
+	} else if (property == config->tv_contrast_property) {
+		*val = state->tv.contrast;
+	} else if (property == config->tv_flicker_reduction_property) {
+		*val = state->tv.flicker_reduction;
+	} else if (property == config->tv_overscan_property) {
+		*val = state->tv.overscan;
+	} else if (property == config->tv_saturation_property) {
+		*val = state->tv.saturation;
+	} else if (property == config->tv_hue_property) {
+		*val = state->tv.hue;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ac9d7d8e0e43..2382d44e5fff 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -194,10 +194,40 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
 				     unsigned int num_formats);
 
 /**
+ * struct drm_tv_connector_state - TV connector related states
+ * @subconnector: selected subconnector
+ * @margins: left/right/top/bottom margins
+ * @mode: TV mode
+ * @brightness: brightness in percent
+ * @contrast: contrast in percent
+ * @flicker_reduction: flicker reduction in percent
+ * @overscan: overscan in percent
+ * @saturation: saturation in percent
+ * @hue: hue in percent
+ */
+struct drm_tv_connector_state {
+	int subconnector;
+	struct {
+		int left;
+		int right;
+		int top;
+		int bottom;
+	} margins;
+	int mode;
+	int brightness;
+	int contrast;
+	int flicker_reduction;
+	int overscan;
+	int saturation;
+	int hue;
+};
+
+/**
  * struct drm_connector_state - mutable connector state
  * @connector: backpointer to the connector
  * @best_encoder: can be used by helpers and drivers to select the encoder
  * @state: backpointer to global drm_atomic_state
+ * @tv: TV connector state
  */
 struct drm_connector_state {
 	struct drm_connector *connector;
@@ -213,6 +243,8 @@ struct drm_connector_state {
 	struct drm_encoder *best_encoder;
 
 	struct drm_atomic_state *state;
+
+	struct drm_tv_connector_state tv;
 };
 
 /**
-- 
2.10.2

Re: [PATCH 2/6] drm: Add TV connector states to drm_connector_state

[Daniel Vetter]

On Tue, Nov 29, 2016 at 10:41:58AM -0800, Eric Anholt wrote:
> From: Boris Brezillon <boris.brezillon@free-electrons.com>
> 
> Some generic TV connector properties are exposed in drm_mode_config, but
> they are currently handled independently in each DRM encoder driver.
> 
> Extend the drm_connector_state to store TV related states, and modify the
> drm_atomic_connector_{set,get}_property() helpers to fill the connector
> state accordingly.
> 
> Each driver is then responsible for checking and applying the new config
> in its ->atomic_mode_{check,set}() operations.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/drm_atomic.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h  | 32 ++++++++++++++++++++++++++++
>  2 files changed, 82 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 23739609427d..02b0668f51e1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -986,12 +986,38 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
>  		 * now?) atomic writes to DPMS property:
>  		 */
>  		return -EINVAL;
> +	} else if (property == config->tv_select_subconnector_property) {
> +		state->tv.subconnector = val;
> +	} else if (property == config->tv_left_margin_property) {
> +		state->tv.margins.left = val;
> +	} else if (property == config->tv_right_margin_property) {
> +		state->tv.margins.right = val;
> +	} else if (property == config->tv_top_margin_property) {
> +		state->tv.margins.bottom = val;

s/bottom/top/

> +	} else if (property == config->tv_bottom_margin_property) {
> +		state->tv.margins.right = val;

s/right/bottom/

> +	} else if (property == config->tv_mode_property) {
> +		state->tv.mode = val;
> +	} else if (property == config->tv_brightness_property) {
> +		state->tv.brightness = val;
> +	} else if (property == config->tv_contrast_property) {
> +		state->tv.contrast = val;
> +	} else if (property == config->tv_flicker_reduction_property) {
> +		state->tv.flicker_reduction = val;
> +	} else if (property == config->tv_overscan_property) {
> +		state->tv.overscan = val;
> +	} else if (property == config->tv_saturation_property) {
> +		state->tv.saturation = val;
> +	} else if (property == config->tv_hue_property) {
> +		state->tv.hue = val;
>  	} else if (connector->funcs->atomic_set_property) {
>  		return connector->funcs->atomic_set_property(connector,
>  				state, property, val);
>  	} else {
>  		return -EINVAL;
>  	}
> +
> +	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_connector_set_property);
>  
> @@ -1022,6 +1048,30 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = (state->crtc) ? state->crtc->base.id : 0;
>  	} else if (property == config->dpms_property) {
>  		*val = connector->dpms;
> +	} else if (property == config->tv_select_subconnector_property) {
> +		*val = state->tv.subconnector;
> +	} else if (property == config->tv_left_margin_property) {
> +		*val = state->tv.margins.left;
> +	} else if (property == config->tv_right_margin_property) {
> +		*val = state->tv.margins.right;
> +	} else if (property == config->tv_top_margin_property) {
> +		*val = state->tv.margins.bottom;

s/bottom/top/
> +	} else if (property == config->tv_bottom_margin_property) {
> +		*val = state->tv.margins.right;

s/right/bottom/

> +	} else if (property == config->tv_mode_property) {
> +		*val = state->tv.mode;
> +	} else if (property == config->tv_brightness_property) {
> +		*val = state->tv.brightness;
> +	} else if (property == config->tv_contrast_property) {
> +		*val = state->tv.contrast;
> +	} else if (property == config->tv_flicker_reduction_property) {
> +		*val = state->tv.flicker_reduction;
> +	} else if (property == config->tv_overscan_property) {
> +		*val = state->tv.overscan;
> +	} else if (property == config->tv_saturation_property) {
> +		*val = state->tv.saturation;
> +	} else if (property == config->tv_hue_property) {
> +		*val = state->tv.hue;
>  	} else if (connector->funcs->atomic_get_property) {
>  		return connector->funcs->atomic_get_property(connector,
>  				state, property, val);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ac9d7d8e0e43..2382d44e5fff 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -194,10 +194,40 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
>  				     unsigned int num_formats);
>  
>  /**
> + * struct drm_tv_connector_state - TV connector related states
> + * @subconnector: selected subconnector
> + * @margins: left/right/top/bottom margins
> + * @mode: TV mode
> + * @brightness: brightness in percent
> + * @contrast: contrast in percent
> + * @flicker_reduction: flicker reduction in percent
> + * @overscan: overscan in percent
> + * @saturation: saturation in percent
> + * @hue: hue in percent
> + */
> +struct drm_tv_connector_state {
> +	int subconnector;

I think an explicit enum for the possible values would be nice. Slightly
annoying/fragile since it needs to match the uabi ones, but we I think
there's a slight preference to explicit enums internally.

> +	struct {
> +		int left;
> +		int right;
> +		int top;
> +		int bottom;
> +	} margins;
> +	int mode;
> +	int brightness;
> +	int contrast;
> +	int flicker_reduction;
> +	int overscan;
> +	int saturation;
> +	int hue;

All the above properties are unsigned when exposed to userspace. I think
for consistency would be good to make them match on the kernel side too.

With the above issues addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

And since no one else seems to work on an atomic TV encoder feel free to
just stash into the next vc4 pull request.
-Daniel

> +};
> +
> +/**
>   * struct drm_connector_state - mutable connector state
>   * @connector: backpointer to the connector
>   * @best_encoder: can be used by helpers and drivers to select the encoder
>   * @state: backpointer to global drm_atomic_state
> + * @tv: TV connector state
>   */
>  struct drm_connector_state {
>  	struct drm_connector *connector;
> @@ -213,6 +243,8 @@ struct drm_connector_state {
>  	struct drm_encoder *best_encoder;
>  
>  	struct drm_atomic_state *state;
> +
> +	struct drm_tv_connector_state tv;
>  };
>  
>  /**
> -- 
> 2.10.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/6] drm: Add TV connector states to drm_connector_state

[Boris Brezillon]

Hi Daniel,

On Tue, 29 Nov 2016 21:14:10 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Nov 29, 2016 at 10:41:58AM -0800, Eric Anholt wrote:
> > From: Boris Brezillon <boris.brezillon@free-electrons.com>
> > 
> > Some generic TV connector properties are exposed in drm_mode_config, but
> > they are currently handled independently in each DRM encoder driver.
> > 
> > Extend the drm_connector_state to store TV related states, and modify the
> > drm_atomic_connector_{set,get}_property() helpers to fill the connector
> > state accordingly.
> > 
> > Each driver is then responsible for checking and applying the new config
> > in its ->atomic_mode_{check,set}() operations.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > Signed-off-by: Eric Anholt <eric@anholt.net>
> > ---
> >  drivers/gpu/drm/drm_atomic.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_connector.h  | 32 ++++++++++++++++++++++++++++
> >  2 files changed, 82 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 23739609427d..02b0668f51e1 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -986,12 +986,38 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
> >  		 * now?) atomic writes to DPMS property:
> >  		 */
> >  		return -EINVAL;
> > +	} else if (property == config->tv_select_subconnector_property) {
> > +		state->tv.subconnector = val;
> > +	} else if (property == config->tv_left_margin_property) {
> > +		state->tv.margins.left = val;
> > +	} else if (property == config->tv_right_margin_property) {
> > +		state->tv.margins.right = val;
> > +	} else if (property == config->tv_top_margin_property) {
> > +		state->tv.margins.bottom = val;  
> 
> s/bottom/top/
> 
> > +	} else if (property == config->tv_bottom_margin_property) {
> > +		state->tv.margins.right = val;  
> 
> s/right/bottom/
> 
> > +	} else if (property == config->tv_mode_property) {
> > +		state->tv.mode = val;
> > +	} else if (property == config->tv_brightness_property) {
> > +		state->tv.brightness = val;
> > +	} else if (property == config->tv_contrast_property) {
> > +		state->tv.contrast = val;
> > +	} else if (property == config->tv_flicker_reduction_property) {
> > +		state->tv.flicker_reduction = val;
> > +	} else if (property == config->tv_overscan_property) {
> > +		state->tv.overscan = val;
> > +	} else if (property == config->tv_saturation_property) {
> > +		state->tv.saturation = val;
> > +	} else if (property == config->tv_hue_property) {
> > +		state->tv.hue = val;
> >  	} else if (connector->funcs->atomic_set_property) {
> >  		return connector->funcs->atomic_set_property(connector,
> >  				state, property, val);
> >  	} else {
> >  		return -EINVAL;
> >  	}
> > +
> > +	return 0;
> >  }
> >  EXPORT_SYMBOL(drm_atomic_connector_set_property);
> >  
> > @@ -1022,6 +1048,30 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  		*val = (state->crtc) ? state->crtc->base.id : 0;
> >  	} else if (property == config->dpms_property) {
> >  		*val = connector->dpms;
> > +	} else if (property == config->tv_select_subconnector_property) {
> > +		*val = state->tv.subconnector;
> > +	} else if (property == config->tv_left_margin_property) {
> > +		*val = state->tv.margins.left;
> > +	} else if (property == config->tv_right_margin_property) {
> > +		*val = state->tv.margins.right;
> > +	} else if (property == config->tv_top_margin_property) {
> > +		*val = state->tv.margins.bottom;  
> 
> s/bottom/top/
> > +	} else if (property == config->tv_bottom_margin_property) {
> > +		*val = state->tv.margins.right;  
> 
> s/right/bottom/

Oops. I'll fix those typos.

> 
> > +	} else if (property == config->tv_mode_property) {
> > +		*val = state->tv.mode;
> > +	} else if (property == config->tv_brightness_property) {
> > +		*val = state->tv.brightness;
> > +	} else if (property == config->tv_contrast_property) {
> > +		*val = state->tv.contrast;
> > +	} else if (property == config->tv_flicker_reduction_property) {
> > +		*val = state->tv.flicker_reduction;
> > +	} else if (property == config->tv_overscan_property) {
> > +		*val = state->tv.overscan;
> > +	} else if (property == config->tv_saturation_property) {
> > +		*val = state->tv.saturation;
> > +	} else if (property == config->tv_hue_property) {
> > +		*val = state->tv.hue;
> >  	} else if (connector->funcs->atomic_get_property) {
> >  		return connector->funcs->atomic_get_property(connector,
> >  				state, property, val);
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index ac9d7d8e0e43..2382d44e5fff 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -194,10 +194,40 @@ int drm_display_info_set_bus_formats(struct drm_display_info *info,
> >  				     unsigned int num_formats);
> >  
> >  /**
> > + * struct drm_tv_connector_state - TV connector related states
> > + * @subconnector: selected subconnector
> > + * @margins: left/right/top/bottom margins
> > + * @mode: TV mode
> > + * @brightness: brightness in percent
> > + * @contrast: contrast in percent
> > + * @flicker_reduction: flicker reduction in percent
> > + * @overscan: overscan in percent
> > + * @saturation: saturation in percent
> > + * @hue: hue in percent
> > + */
> > +struct drm_tv_connector_state {
> > +	int subconnector;  
> 
> I think an explicit enum for the possible values would be nice. Slightly
> annoying/fragile since it needs to match the uabi ones, but we I think
> there's a slight preference to explicit enums internally.

Okay.

> 
> > +	struct {
> > +		int left;
> > +		int right;
> > +		int top;
> > +		int bottom;
> > +	} margins;
> > +	int mode;
> > +	int brightness;
> > +	int contrast;
> > +	int flicker_reduction;
> > +	int overscan;
> > +	int saturation;
> > +	int hue;  
> 
> All the above properties are unsigned when exposed to userspace. I think
> for consistency would be good to make them match on the kernel side too.

Sure.

> 
> With the above issues addressed:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> And since no one else seems to work on an atomic TV encoder feel free to
> just stash into the next vc4 pull request.

Thanks for the review.

Boris

[PATCH 3/6] drm/vc4: Add support for the VEC (Video Encoder) IP

[Eric Anholt]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

The VEC IP is a TV DAC, providing support for PAL and NTSC standards.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/Makefile      |   1 +
 drivers/gpu/drm/vc4/vc4_debugfs.c |   1 +
 drivers/gpu/drm/vc4/vc4_drv.c     |   1 +
 drivers/gpu/drm/vc4/vc4_drv.h     |   5 +
 drivers/gpu/drm/vc4/vc4_vec.c     | 657 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 665 insertions(+)
 create mode 100644 drivers/gpu/drm/vc4/vc4_vec.c

diff --git a/drivers/gpu/drm/vc4/Makefile b/drivers/gpu/drm/vc4/Makefile
index fb77db755e0a..7757f69a8a77 100644
--- a/drivers/gpu/drm/vc4/Makefile
+++ b/drivers/gpu/drm/vc4/Makefile
@@ -11,6 +11,7 @@ vc4-y := \
 	vc4_kms.o \
 	vc4_gem.o \
 	vc4_hdmi.o \
+	vc4_vec.o \
 	vc4_hvs.o \
 	vc4_irq.o \
 	vc4_plane.o \
diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index 245115d49c46..caf817bac885 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -19,6 +19,7 @@ static const struct drm_info_list vc4_debugfs_list[] = {
 	{"bo_stats", vc4_bo_stats_debugfs, 0},
 	{"dpi_regs", vc4_dpi_debugfs_regs, 0},
 	{"hdmi_regs", vc4_hdmi_debugfs_regs, 0},
+	{"vec_regs", vc4_vec_debugfs_regs, 0},
 	{"hvs_regs", vc4_hvs_debugfs_regs, 0},
 	{"crtc0_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)0},
 	{"crtc1_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)1},
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 86aabf6d0f79..df3da26a849e 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -294,6 +294,7 @@ static const struct component_master_ops vc4_drm_ops = {
 
 static struct platform_driver *const component_drivers[] = {
 	&vc4_hdmi_driver,
+	&vc4_vec_driver,
 	&vc4_dpi_driver,
 	&vc4_hvs_driver,
 	&vc4_crtc_driver,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index b6049cc25675..162821796bf3 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -17,6 +17,7 @@ struct vc4_dev {
 	struct vc4_crtc *crtc[3];
 	struct vc4_v3d *v3d;
 	struct vc4_dpi *dpi;
+	struct vc4_vec *vec;
 
 	struct drm_fbdev_cma *fbdev;
 
@@ -486,6 +487,10 @@ int vc4_queue_seqno_cb(struct drm_device *dev,
 extern struct platform_driver vc4_hdmi_driver;
 int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused);
 
+/* vc4_hdmi.c */
+extern struct platform_driver vc4_vec_driver;
+int vc4_vec_debugfs_regs(struct seq_file *m, void *unused);
+
 /* vc4_irq.c */
 irqreturn_t vc4_irq(int irq, void *arg);
 void vc4_irq_preinstall(struct drm_device *dev);
diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
new file mode 100644
index 000000000000..32bb8ef985fb
--- /dev/null
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -0,0 +1,657 @@
+/*
+ * Copyright (C) 2016 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/**
+ * DOC: VC4 SDTV module
+ */
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_panel.h>
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/of_graph.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+
+#include "vc4_drv.h"
+#include "vc4_regs.h"
+
+/* WSE Registers */
+#define VEC_WSE_RESET			0xc0
+
+#define VEC_WSE_CONTROL			0xc4
+#define VEC_WSE_WSS_ENABLE		BIT(7)
+
+#define VEC_WSE_WSS_DATA		0xc8
+#define VEC_WSE_VPS_DATA1		0xcc
+#define VEC_WSE_VPS_CONTROL		0xd0
+
+/* VEC Registers */
+#define VEC_REVID			0x100
+
+#define VEC_CONFIG0			0x104
+#define VEC_CONFIG0_YDEL_MASK		GENMASK(28, 26)
+#define VEC_CONFIG0_YDEL(x)		((x) << 26)
+#define VEC_CONFIG0_CDEL_MASK		GENMASK(25, 24)
+#define VEC_CONFIG0_CDEL(x)		((x) << 24)
+#define VEC_CONFIG0_PBPR_FIL		BIT(18)
+#define VEC_CONFIG0_CHROMA_GAIN_MASK	GENMASK(17, 16)
+#define VEC_CONFIG0_CHROMA_GAIN_UNITY	(0 << 16)
+#define VEC_CONFIG0_CHROMA_GAIN_1_32	(1 << 16)
+#define VEC_CONFIG0_CHROMA_GAIN_1_16	(2 << 16)
+#define VEC_CONFIG0_CHROMA_GAIN_1_8	(3 << 16)
+#define VEC_CONFIG0_CBURST_GAIN_MASK	GENMASK(14, 13)
+#define VEC_CONFIG0_CBURST_GAIN_UNITY	(0 << 13)
+#define VEC_CONFIG0_CBURST_GAIN_1_128	(1 << 13)
+#define VEC_CONFIG0_CBURST_GAIN_1_64	(2 << 13)
+#define VEC_CONFIG0_CBURST_GAIN_1_32	(3 << 13)
+#define VEC_CONFIG0_CHRBW1		BIT(11)
+#define VEC_CONFIG0_CHRBW0		BIT(10)
+#define VEC_CONFIG0_SYNCDIS		BIT(9)
+#define VEC_CONFIG0_BURDIS		BIT(8)
+#define VEC_CONFIG0_CHRDIS		BIT(7)
+#define VEC_CONFIG0_PDEN		BIT(6)
+#define VEC_CONFIG0_YCDELAY		BIT(4)
+#define VEC_CONFIG0_RAMPEN		BIT(2)
+#define VEC_CONFIG0_YCDIS		BIT(2)
+#define VEC_CONFIG0_STD_MASK		GENMASK(1, 0)
+#define VEC_CONFIG0_NTSC_STD		0
+#define VEC_CONFIG0_PAL_BDGHI_STD	1
+#define VEC_CONFIG0_PAL_N_STD		3
+
+#define VEC_SCHPH			0x108
+#define VEC_SOFT_RESET			0x10c
+#define VEC_CLMP0_START			0x144
+#define VEC_CLMP0_END			0x148
+#define VEC_FREQ3_2			0x180
+#define VEC_FREQ1_0			0x184
+
+#define VEC_CONFIG1			0x188
+#define VEC_CONFIG_VEC_RESYNC_OFF	BIT(18)
+#define VEC_CONFIG_RGB219		BIT(17)
+#define VEC_CONFIG_CBAR_EN		BIT(16)
+#define VEC_CONFIG_TC_OBB		BIT(15)
+#define VEC_CONFIG1_OUTPUT_MODE_MASK	GENMASK(12, 10)
+#define VEC_CONFIG1_C_Y_CVBS		(0 << 10)
+#define VEC_CONFIG1_CVBS_Y_C		(1 << 10)
+#define VEC_CONFIG1_PR_Y_PB		(2 << 10)
+#define VEC_CONFIG1_RGB			(4 << 10)
+#define VEC_CONFIG1_Y_C_CVBS		(5 << 10)
+#define VEC_CONFIG1_C_CVBS_Y		(6 << 10)
+#define VEC_CONFIG1_C_CVBS_CVBS		(7 << 10)
+#define VEC_CONFIG1_DIS_CHR		BIT(9)
+#define VEC_CONFIG1_DIS_LUMA		BIT(8)
+#define VEC_CONFIG1_YCBCR_IN		BIT(6)
+#define VEC_CONFIG1_DITHER_TYPE_LFSR	0
+#define VEC_CONFIG1_DITHER_TYPE_COUNTER	BIT(5)
+#define VEC_CONFIG1_DITHER_EN		BIT(4)
+#define VEC_CONFIG1_CYDELAY		BIT(3)
+#define VEC_CONFIG1_LUMADIS		BIT(2)
+#define VEC_CONFIG1_COMPDIS		BIT(1)
+#define VEC_CONFIG1_CUSTOM_FREQ		BIT(0)
+
+#define VEC_CONFIG2			0x18c
+#define VEC_CONFIG2_PROG_SCAN		BIT(15)
+#define VEC_CONFIG2_SYNC_ADJ_MASK	GENMASK(14, 12)
+#define VEC_CONFIG2_SYNC_ADJ(x)		(((x) / 2) << 12)
+#define VEC_CONFIG2_PBPR_EN		BIT(10)
+#define VEC_CONFIG2_UV_DIG_DIS		BIT(6)
+#define VEC_CONFIG2_RGB_DIG_DIS		BIT(5)
+#define VEC_CONFIG2_TMUX_MASK		GENMASK(3, 2)
+#define VEC_CONFIG2_TMUX_DRIVE0		(0 << 2)
+#define VEC_CONFIG2_TMUX_RG_COMP	(1 << 2)
+#define VEC_CONFIG2_TMUX_UV_YC		(2 << 2)
+#define VEC_CONFIG2_TMUX_SYNC_YC	(3 << 2)
+
+#define VEC_INTERRUPT_CONTROL		0x190
+#define VEC_INTERRUPT_STATUS		0x194
+#define VEC_FCW_SECAM_B			0x198
+#define VEC_SECAM_GAIN_VAL		0x19c
+
+#define VEC_CONFIG3			0x1a0
+#define VEC_CONFIG3_HORIZ_LEN_STD	(0 << 0)
+#define VEC_CONFIG3_HORIZ_LEN_MPEG1_SIF	(1 << 0)
+#define VEC_CONFIG3_SHAPE_NON_LINEAR	BIT(1)
+
+#define VEC_STATUS0			0x200
+#define VEC_MASK0			0x204
+
+#define VEC_CFG				0x208
+#define VEC_CFG_SG_MODE_MASK		GENMASK(6, 5)
+#define VEC_CFG_SG_MODE(x)		((x) << 5)
+#define VEC_CFG_SG_EN			BIT(4)
+#define VEC_CFG_VEC_EN			BIT(3)
+#define VEC_CFG_MB_EN			BIT(2)
+#define VEC_CFG_ENABLE			BIT(1)
+#define VEC_CFG_TB_EN			BIT(0)
+
+#define VEC_DAC_TEST			0x20c
+
+#define VEC_DAC_CONFIG			0x210
+#define VEC_DAC_CONFIG_LDO_BIAS_CTRL(x)	((x) << 24)
+#define VEC_DAC_CONFIG_DRIVER_CTRL(x)	((x) << 16)
+#define VEC_DAC_CONFIG_DAC_CTRL(x)	(x)
+
+#define VEC_DAC_MISC			0x214
+#define VEC_DAC_MISC_VCD_CTRL_MASK	GENMASK(31, 16)
+#define VEC_DAC_MISC_VCD_CTRL(x)	((x) << 16)
+#define VEC_DAC_MISC_VID_ACT		BIT(8)
+#define VEC_DAC_MISC_VCD_PWRDN		BIT(6)
+#define VEC_DAC_MISC_BIAS_PWRDN		BIT(5)
+#define VEC_DAC_MISC_DAC_PWRDN		BIT(2)
+#define VEC_DAC_MISC_LDO_PWRDN		BIT(1)
+#define VEC_DAC_MISC_DAC_RST_N		BIT(0)
+
+
+/* General VEC hardware state. */
+struct vc4_vec {
+	struct platform_device *pdev;
+
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+
+	void __iomem *regs;
+
+	struct clk *clock;
+
+	const struct vc4_vec_tv_mode *tv_mode;
+};
+
+#define VEC_READ(offset) readl(vec->regs + (offset))
+#define VEC_WRITE(offset, val) writel(val, vec->regs + (offset))
+
+/* VC4 VEC encoder KMS struct */
+struct vc4_vec_encoder {
+	struct vc4_encoder base;
+	struct vc4_vec *vec;
+};
+
+static inline struct vc4_vec_encoder *
+to_vc4_vec_encoder(struct drm_encoder *encoder)
+{
+	return container_of(encoder, struct vc4_vec_encoder, base.base);
+}
+
+/* VC4 VEC connector KMS struct */
+struct vc4_vec_connector {
+	struct drm_connector base;
+	struct vc4_vec *vec;
+
+	/* Since the connector is attached to just the one encoder,
+	 * this is the reference to it so we can do the best_encoder()
+	 * hook.
+	 */
+	struct drm_encoder *encoder;
+};
+
+static inline struct vc4_vec_connector *
+to_vc4_vec_connector(struct drm_connector *connector)
+{
+	return container_of(connector, struct vc4_vec_connector, base);
+}
+
+enum vc4_vec_tv_mode_id {
+	VC4_VEC_TV_MODE_NTSC,
+	VC4_VEC_TV_MODE_NTSC_J,
+	VC4_VEC_TV_MODE_PAL,
+	VC4_VEC_TV_MODE_PAL_M,
+};
+
+struct vc4_vec_tv_mode {
+	const struct drm_display_mode *mode;
+	void (*mode_set)(struct vc4_vec *vec);
+};
+
+#define VEC_REG(reg) { reg, #reg }
+static const struct {
+	u32 reg;
+	const char *name;
+} vec_regs[] = {
+	VEC_REG(VEC_WSE_CONTROL),
+	VEC_REG(VEC_WSE_WSS_DATA),
+	VEC_REG(VEC_WSE_VPS_DATA1),
+	VEC_REG(VEC_WSE_VPS_CONTROL),
+	VEC_REG(VEC_REVID),
+	VEC_REG(VEC_CONFIG0),
+	VEC_REG(VEC_SCHPH),
+	VEC_REG(VEC_CLMP0_START),
+	VEC_REG(VEC_CLMP0_END),
+	VEC_REG(VEC_FREQ3_2),
+	VEC_REG(VEC_FREQ1_0),
+	VEC_REG(VEC_CONFIG1),
+	VEC_REG(VEC_CONFIG2),
+	VEC_REG(VEC_INTERRUPT_CONTROL),
+	VEC_REG(VEC_INTERRUPT_STATUS),
+	VEC_REG(VEC_FCW_SECAM_B),
+	VEC_REG(VEC_SECAM_GAIN_VAL),
+	VEC_REG(VEC_CONFIG3),
+	VEC_REG(VEC_STATUS0),
+	VEC_REG(VEC_MASK0),
+	VEC_REG(VEC_CFG),
+	VEC_REG(VEC_DAC_TEST),
+	VEC_REG(VEC_DAC_CONFIG),
+	VEC_REG(VEC_DAC_MISC),
+};
+
+#ifdef CONFIG_DEBUG_FS
+int vc4_vec_debugfs_regs(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = (struct drm_info_node *)m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	struct vc4_vec *vec = vc4->vec;
+	int i;
+
+	if (!vec)
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(vec_regs); i++) {
+		seq_printf(m, "%s (0x%04x): 0x%08x\n",
+			   vec_regs[i].name, vec_regs[i].reg,
+			   VEC_READ(vec_regs[i].reg));
+	}
+
+	return 0;
+}
+#endif
+
+static void vc4_vec_ntsc_mode_set(struct vc4_vec *vec)
+{
+	VEC_WRITE(VEC_CONFIG0, VEC_CONFIG0_NTSC_STD | VEC_CONFIG0_PDEN);
+	VEC_WRITE(VEC_CONFIG1, VEC_CONFIG1_C_CVBS_CVBS);
+}
+
+static void vc4_vec_ntsc_j_mode_set(struct vc4_vec *vec)
+{
+	VEC_WRITE(VEC_CONFIG0, VEC_CONFIG0_NTSC_STD);
+	VEC_WRITE(VEC_CONFIG1, VEC_CONFIG1_C_CVBS_CVBS);
+}
+
+static const struct drm_display_mode ntsc_mode = {
+	DRM_MODE("720x480", DRM_MODE_TYPE_DRIVER, 13500,
+		 720, 720 + 14, 720 + 14 + 64, 720 + 14 + 64 + 60, 0,
+		 480, 480 + 3, 480 + 3 + 3, 480 + 3 + 3 + 16, 0,
+		 DRM_MODE_FLAG_INTERLACE)
+};
+
+static void vc4_vec_pal_mode_set(struct vc4_vec *vec)
+{
+	VEC_WRITE(VEC_CONFIG0, VEC_CONFIG0_PAL_BDGHI_STD);
+	VEC_WRITE(VEC_CONFIG1, VEC_CONFIG1_C_CVBS_CVBS);
+}
+
+static void vc4_vec_pal_m_mode_set(struct vc4_vec *vec)
+{
+	VEC_WRITE(VEC_CONFIG0, VEC_CONFIG0_PAL_BDGHI_STD);
+	VEC_WRITE(VEC_CONFIG1,
+		  VEC_CONFIG1_C_CVBS_CVBS | VEC_CONFIG1_CUSTOM_FREQ);
+	VEC_WRITE(VEC_FREQ3_2, 0x223b);
+	VEC_WRITE(VEC_FREQ1_0, 0x61d1);
+}
+
+static const struct drm_display_mode pal_mode = {
+	DRM_MODE("720x576", DRM_MODE_TYPE_DRIVER, 13500,
+		 720, 720 + 20, 720 + 20 + 64, 720 + 20 + 64 + 60, 0,
+		 576, 576 + 2, 576 + 2 + 3, 576 + 2 + 3 + 20, 0,
+		 DRM_MODE_FLAG_INTERLACE)
+};
+
+static const struct vc4_vec_tv_mode vc4_vec_tv_modes[] = {
+	[VC4_VEC_TV_MODE_NTSC] = {
+		.mode = &ntsc_mode,
+		.mode_set = vc4_vec_ntsc_mode_set,
+	},
+	[VC4_VEC_TV_MODE_NTSC_J] = {
+		.mode = &ntsc_mode,
+		.mode_set = vc4_vec_ntsc_j_mode_set,
+	},
+	[VC4_VEC_TV_MODE_PAL] = {
+		.mode = &pal_mode,
+		.mode_set = vc4_vec_pal_mode_set,
+	},
+	[VC4_VEC_TV_MODE_PAL_M] = {
+		.mode = &pal_mode,
+		.mode_set = vc4_vec_pal_m_mode_set,
+	},
+};
+
+static enum drm_connector_status
+vc4_vec_connector_detect(struct drm_connector *connector, bool force)
+{
+	return connector_status_unknown;
+}
+
+static void vc4_vec_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static int vc4_vec_connector_get_modes(struct drm_connector *connector)
+{
+	struct drm_connector_state *state = connector->state;
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(connector->dev,
+				  vc4_vec_tv_modes[state->tv.mode].mode);
+	if (!mode) {
+		DRM_ERROR("Failed to create a new display mode\n");
+		return -ENOMEM;
+	}
+
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_connector_funcs vc4_vec_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.detect = vc4_vec_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.set_property = drm_atomic_helper_connector_set_property,
+	.destroy = vc4_vec_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static const struct drm_connector_helper_funcs vc4_vec_connector_helper_funcs = {
+	.get_modes = vc4_vec_connector_get_modes,
+};
+
+static struct drm_connector *vc4_vec_connector_init(struct drm_device *dev,
+						    struct vc4_vec *vec)
+{
+	struct drm_connector *connector = NULL;
+	struct vc4_vec_connector *vec_connector;
+
+	vec_connector = devm_kzalloc(dev->dev, sizeof(*vec_connector),
+				     GFP_KERNEL);
+	if (!vec_connector)
+		return ERR_PTR(-ENOMEM);
+
+	connector = &vec_connector->base;
+	connector->interlace_allowed = true;
+
+	vec_connector->encoder = vec->encoder;
+	vec_connector->vec = vec;
+
+	drm_connector_init(dev, connector, &vc4_vec_connector_funcs,
+			   DRM_MODE_CONNECTOR_Composite);
+	drm_connector_helper_add(connector, &vc4_vec_connector_helper_funcs);
+
+	drm_object_attach_property(&connector->base,
+				   dev->mode_config.tv_mode_property,
+				   VC4_VEC_TV_MODE_NTSC);
+	vec->tv_mode = &vc4_vec_tv_modes[VC4_VEC_TV_MODE_NTSC];
+
+	drm_mode_connector_attach_encoder(connector, vec->encoder);
+
+	return connector;
+}
+
+static const struct drm_encoder_funcs vc4_vec_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static void vc4_vec_encoder_disable(struct drm_encoder *encoder)
+{
+	struct vc4_vec_encoder *vc4_vec_encoder = to_vc4_vec_encoder(encoder);
+	struct vc4_vec *vec = vc4_vec_encoder->vec;
+	int ret;
+
+	VEC_WRITE(VEC_CFG, 0);
+	VEC_WRITE(VEC_DAC_MISC,
+		  VEC_DAC_MISC_VCD_PWRDN |
+		  VEC_DAC_MISC_BIAS_PWRDN |
+		  VEC_DAC_MISC_DAC_PWRDN |
+		  VEC_DAC_MISC_LDO_PWRDN);
+
+	clk_disable_unprepare(vec->clock);
+
+	ret = pm_runtime_put(&vec->pdev->dev);
+	if (ret < 0) {
+		DRM_ERROR("Failed to release power domain: %d\n", ret);
+		return;
+	}
+}
+
+static void vc4_vec_encoder_enable(struct drm_encoder *encoder)
+{
+	struct vc4_vec_encoder *vc4_vec_encoder = to_vc4_vec_encoder(encoder);
+	struct vc4_vec *vec = vc4_vec_encoder->vec;
+	int ret;
+
+	ret = pm_runtime_get_sync(&vec->pdev->dev);
+	if (ret < 0) {
+		DRM_ERROR("Failed to retain power domain: %d\n", ret);
+		return;
+	}
+
+	/*
+	 * We need to set the clock rate each time we enable the encoder
+	 * because there's a chance we share the same parent with the HDMI
+	 * clock, and both drivers are requesting different rates.
+	 * The good news is, these 2 encoders cannot be enabled at the same
+	 * time, thus preventing incompatible rate requests.
+	 */
+	ret = clk_set_rate(vec->clock, 108000000);
+	if (ret) {
+		DRM_ERROR("Failed to set clock rate: %d\n", ret);
+		return;
+	}
+
+	ret = clk_prepare_enable(vec->clock);
+	if (ret) {
+		DRM_ERROR("Failed to turn on core clock: %d\n", ret);
+		return;
+	}
+
+	/* Reset the different blocks */
+	VEC_WRITE(VEC_WSE_RESET, 1);
+	VEC_WRITE(VEC_SOFT_RESET, 1);
+
+	/* Disable the CGSM-A and WSE blocks */
+	VEC_WRITE(VEC_WSE_CONTROL, 0);
+
+	/* Write config common to all modes. */
+
+	/*
+	 * Color subcarrier phase: phase = 360 * SCHPH / 256.
+	 * 0x28 <=> 39.375 deg.
+	 */
+	VEC_WRITE(VEC_SCHPH, 0x28);
+
+	/*
+	 * Reset to default values.
+	 */
+	VEC_WRITE(VEC_CLMP0_START, 0xac);
+	VEC_WRITE(VEC_CLMP0_END, 0xec);
+	VEC_WRITE(VEC_CONFIG2,
+		  VEC_CONFIG2_UV_DIG_DIS | VEC_CONFIG2_RGB_DIG_DIS);
+	VEC_WRITE(VEC_CONFIG3, VEC_CONFIG3_HORIZ_LEN_STD);
+	VEC_WRITE(VEC_DAC_CONFIG,
+		  VEC_DAC_CONFIG_DAC_CTRL(0xc) |
+		  VEC_DAC_CONFIG_DRIVER_CTRL(0xc) |
+		  VEC_DAC_CONFIG_LDO_BIAS_CTRL(0x46));
+
+	/* Mask all interrupts. */
+	VEC_WRITE(VEC_MASK0, 0);
+
+	vec->tv_mode->mode_set(vec);
+
+	VEC_WRITE(VEC_DAC_MISC,
+		  VEC_DAC_MISC_VID_ACT | VEC_DAC_MISC_DAC_RST_N);
+	VEC_WRITE(VEC_CFG, VEC_CFG_VEC_EN);
+}
+
+
+static bool vc4_vec_encoder_mode_fixup(struct drm_encoder *encoder,
+				       const struct drm_display_mode *mode,
+				       struct drm_display_mode *adjusted_mode)
+{
+	return true;
+}
+
+static void vc4_vec_encoder_atomic_mode_set(struct drm_encoder *encoder,
+					struct drm_crtc_state *crtc_state,
+					struct drm_connector_state *conn_state)
+{
+	struct vc4_vec_encoder *vc4_vec_encoder = to_vc4_vec_encoder(encoder);
+	struct vc4_vec *vec = vc4_vec_encoder->vec;
+
+	vec->tv_mode = &vc4_vec_tv_modes[conn_state->tv.mode];
+}
+
+static int vc4_vec_encoder_atomic_check(struct drm_encoder *encoder,
+					struct drm_crtc_state *crtc_state,
+					struct drm_connector_state *conn_state)
+{
+	const struct vc4_vec_tv_mode *vec_mode;
+
+	vec_mode = &vc4_vec_tv_modes[conn_state->tv.mode];
+
+	if (conn_state->crtc &&
+	    !drm_mode_equal(vec_mode->mode, &crtc_state->adjusted_mode))
+		return -EINVAL;
+
+	return 0;
+}
+
+static const struct drm_encoder_helper_funcs vc4_vec_encoder_helper_funcs = {
+	.disable = vc4_vec_encoder_disable,
+	.enable = vc4_vec_encoder_enable,
+	.mode_fixup = vc4_vec_encoder_mode_fixup,
+	.atomic_check = vc4_vec_encoder_atomic_check,
+	.atomic_mode_set = vc4_vec_encoder_atomic_mode_set,
+};
+
+static const struct of_device_id vc4_vec_dt_match[] = {
+	{ .compatible = "brcm,bcm2835-vec", .data = NULL },
+	{ /* sentinel */ },
+};
+
+static const char * const tv_mode_names[] = {
+	[VC4_VEC_TV_MODE_NTSC] = "NTSC",
+	[VC4_VEC_TV_MODE_NTSC_J] = "NTSC-J",
+	[VC4_VEC_TV_MODE_PAL] = "PAL",
+	[VC4_VEC_TV_MODE_PAL_M] = "PAL-M",
+};
+
+static int vc4_vec_bind(struct device *dev, struct device *master, void *data)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct drm_device *drm = dev_get_drvdata(master);
+	struct vc4_dev *vc4 = to_vc4_dev(drm);
+	struct vc4_vec *vec;
+	struct vc4_vec_encoder *vc4_vec_encoder;
+	int ret;
+
+	ret = drm_mode_create_tv_properties(drm, ARRAY_SIZE(tv_mode_names),
+					    tv_mode_names);
+	if (ret)
+		return ret;
+
+	vec = devm_kzalloc(dev, sizeof(*vec), GFP_KERNEL);
+	if (!vec)
+		return -ENOMEM;
+
+	vc4_vec_encoder = devm_kzalloc(dev, sizeof(*vc4_vec_encoder),
+				       GFP_KERNEL);
+	if (!vc4_vec_encoder)
+		return -ENOMEM;
+	vc4_vec_encoder->base.type = VC4_ENCODER_TYPE_VEC;
+	vc4_vec_encoder->vec = vec;
+	vec->encoder = &vc4_vec_encoder->base.base;
+
+	vec->pdev = pdev;
+	vec->regs = vc4_ioremap_regs(pdev, 0);
+	if (IS_ERR(vec->regs))
+		return PTR_ERR(vec->regs);
+
+	vec->clock = devm_clk_get(dev, NULL);
+	if (IS_ERR(vec->clock)) {
+		ret = PTR_ERR(vec->clock);
+		if (ret != -EPROBE_DEFER)
+			DRM_ERROR("Failed to get clock: %d\n", ret);
+		return ret;
+	}
+
+	pm_runtime_enable(dev);
+
+	drm_encoder_init(drm, vec->encoder, &vc4_vec_encoder_funcs,
+			 DRM_MODE_ENCODER_TVDAC, NULL);
+	drm_encoder_helper_add(vec->encoder, &vc4_vec_encoder_helper_funcs);
+
+	vec->connector = vc4_vec_connector_init(drm, vec);
+	if (IS_ERR(vec->connector)) {
+		ret = PTR_ERR(vec->connector);
+		goto err_destroy_encoder;
+	}
+
+	dev_set_drvdata(dev, vec);
+
+	vc4->vec = vec;
+
+	return 0;
+
+err_destroy_encoder:
+	drm_encoder_cleanup(vec->encoder);
+	pm_runtime_disable(dev);
+
+	return ret;
+}
+
+static void vc4_vec_unbind(struct device *dev, struct device *master,
+			   void *data)
+{
+	struct drm_device *drm = dev_get_drvdata(master);
+	struct vc4_dev *vc4 = to_vc4_dev(drm);
+	struct vc4_vec *vec = dev_get_drvdata(dev);
+
+	vc4_vec_connector_destroy(vec->connector);
+	drm_encoder_cleanup(vec->encoder);
+	pm_runtime_disable(dev);
+
+	vc4->vec = NULL;
+}
+
+static const struct component_ops vc4_vec_ops = {
+	.bind   = vc4_vec_bind,
+	.unbind = vc4_vec_unbind,
+};
+
+static int vc4_vec_dev_probe(struct platform_device *pdev)
+{
+	return component_add(&pdev->dev, &vc4_vec_ops);
+}
+
+static int vc4_vec_dev_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &vc4_vec_ops);
+	return 0;
+}
+
+struct platform_driver vc4_vec_driver = {
+	.probe = vc4_vec_dev_probe,
+	.remove = vc4_vec_dev_remove,
+	.driver = {
+		.name = "vc4_vec",
+		.of_match_table = vc4_vec_dt_match,
+	},
+};
-- 
2.10.2

[PATCH 4/6] drm/vc4: Document VEC DT binding

[Eric Anholt]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

Document the DT binding for the VEC (Video EnCoder) IP.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
---
 Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
index a5ea451e67fc..e2768703ac2b 100644
--- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
+++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt
@@ -43,6 +43,13 @@ Required properties for DPI:
 - port:		Port node with a single endpoint connecting to the panel
 		  device, as defined in [1]
 
+Required properties for VEC:
+- compatible:	Should be "brcm,bcm2835-vec"
+- reg:		Physical base address and length of the registers
+- clocks:	The core clock the unit runs on
+- interrupts:	The interrupt number
+		  See bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt
+
 Required properties for V3D:
 - compatible:	Should be "brcm,bcm2835-v3d"
 - reg:		Physical base address and length of the V3D's registers
@@ -92,6 +99,13 @@ dpi: dpi@7e208000 {
 	};
 };
 
+vec: vec@7e806000 {
+	compatible = "brcm,bcm2835-vec";
+	reg = <0x7e806000 0x1000>;
+	clocks = <&clocks BCM2835_CLOCK_VEC>;
+	interrupts = <2 27>;
+};
+
 v3d: v3d@7ec00000 {
 	compatible = "brcm,bcm2835-v3d";
 	reg = <0x7ec00000 0x1000>;
-- 
2.10.2

[PATCH 5/6] ARM: bcm/dt: Add VEC node in bcm283x.dtsi

[Eric Anholt]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

Add the VEC (Video EnCoder) node definition in bcm283x.dtsi.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
---
 arch/arm/boot/dts/bcm283x.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index 46d46d894a44..44a9c0539437 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -266,6 +266,14 @@
 			status = "disabled";
 		};
 
+		vec: vec@7e806000 {
+			compatible = "brcm,bcm2835-vec";
+			reg = <0x7e806000 0x1000>;
+			clocks = <&clocks BCM2835_CLOCK_VEC>;
+			interrupts = <2 27>;
+			status = "disabled";
+		};
+
 		pixelvalve@7e807000 {
 			compatible = "brcm,bcm2835-pixelvalve2";
 			reg = <0x7e807000 0x100>;
-- 
2.10.2

[PATCH 6/6] ARM: bcm/dt: Enable the VEC IP on all RaspberryPi boards

[Eric Anholt]

From: Boris Brezillon <boris.brezillon@free-electrons.com>

Enable the VEC IP on all RaspberryPi boards.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
---
 arch/arm/boot/dts/bcm2835-rpi.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index e9b47b2bbc33..8893240da5f6 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -84,3 +84,8 @@
 	power-domains = <&power RPI_POWER_DOMAIN_HDMI>;
 	status = "okay";
 };
+
+&vec {
+	power-domains = <&power RPI_POWER_DOMAIN_VEC>;
+	status = "okay";
+};
-- 
2.10.2