All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: [PATCH 1/3] drm/i915: Convert intel_tv connector properties to atomic, v3.
Date: Wed, 15 Mar 2017 11:08:56 +0100	[thread overview]
Message-ID: <21a8348b-aca9-3c19-3a3d-9ed766053eb9@linux.intel.com> (raw)
In-Reply-To: <20170314183625.GQ31595@intel.com>

As a proof of concept, first try to convert intel_tv, which is a rarely
used connector. It has 5 properties, tv format and 4 margins.

I'm less certain about the state behavior itself, should we pass a size
parameter to intel_connector_alloc instead, so duplicate_state
can be done globally if it can be blindly copied?

Can we also have a atomic_check function for connectors, so the
crtc_state->connectors_changed can be set there? It would be cleaner
and more atomic-like.

To match the legacy behavior, format can be changed by probing just like
in legacy mode.

Changes since v1:
- Add intel_encoder->swap_state to allow updating connector state.
- Add intel_tv->format for detect_mode and mode_valid, updated on atomic commit.
Changes since v2:
- Fix typo in tv_choose_preferred modes function name.
- Assignment of tv properties is done in core, so intel_tv only needs
  a atomic_check function. Thanks Ville!

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  15 +++
 drivers/gpu/drm/i915/intel_drv.h     |   4 +
 drivers/gpu/drm/i915/intel_tv.c      | 200 ++++++++++++++++-------------------
 3 files changed, 110 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ac25c9bc8b81..18b7e7546ee1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12954,6 +12954,20 @@ static void intel_atomic_track_fbs(struct drm_atomic_state *state)
 				  to_intel_plane(plane)->frontbuffer_bit);
 }
 
+static void swap_connector_state(struct drm_atomic_state *state)
+{
+	struct drm_connector *connector;
+	struct drm_connector_state *new_conn_state;
+	int i;
+
+	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
+		struct intel_connector *conn = to_intel_connector(connector);
+
+		if (conn->swap_state)
+			conn->swap_state(conn, new_conn_state);
+	}
+}
+
 /**
  * intel_atomic_commit - commit validated state object
  * @dev: DRM device
@@ -13012,6 +13026,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 		dev_priv->cdclk.logical = intel_state->cdclk.logical;
 		dev_priv->cdclk.actual = intel_state->cdclk.actual;
 	}
+	swap_connector_state(state);
 
 	drm_atomic_state_get(state);
 	INIT_WORK(&state->commit_work,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 937623ff6d7c..b7b93799d288 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -307,6 +307,10 @@ struct intel_connector {
 	 * and active (i.e. dpms ON state). */
 	bool (*get_hw_state)(struct intel_connector *);
 
+	/* Update device state with the new atomic state. */
+	void (*swap_state)(struct intel_connector *,
+			     struct drm_connector_state *);
+
 	/* Panel info for eDP and LVDS */
 	struct intel_panel panel;
 
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 6ed1a3ce47b7..eeed17e6741a 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -48,8 +48,6 @@ struct intel_tv {
 	struct intel_encoder base;
 
 	int type;
-	const char *tv_format;
-	int margin[4];
 	u32 save_TV_H_CTL_1;
 	u32 save_TV_H_CTL_2;
 	u32 save_TV_H_CTL_3;
@@ -83,6 +81,8 @@ struct intel_tv {
 
 	u32 save_TV_DAC;
 	u32 save_TV_CTL;
+
+	int format;
 };
 
 struct video_levels {
@@ -848,6 +848,15 @@ intel_tv_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe)
 }
 
 static void
+intel_tv_swap_state(struct intel_connector *connector,
+		    struct drm_connector_state *conn_state)
+{
+	struct intel_tv *intel_tv = intel_attached_tv(&connector->base);
+
+	intel_tv->format = conn_state->tv.mode;
+}
+
+static void
 intel_enable_tv(struct intel_encoder *encoder,
 		struct intel_crtc_state *pipe_config,
 		struct drm_connector_state *conn_state)
@@ -873,32 +882,25 @@ intel_disable_tv(struct intel_encoder *encoder,
 	I915_WRITE(TV_CTL, I915_READ(TV_CTL) & ~TV_ENC_ENABLE);
 }
 
-static const struct tv_mode *
-intel_tv_mode_lookup(const char *tv_format)
+static const struct tv_mode *intel_tv_mode_find(struct drm_connector_state *conn_state)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(tv_modes); i++) {
-		const struct tv_mode *tv_mode = &tv_modes[i];
+	int format = conn_state->tv.mode;
 
-		if (!strcmp(tv_format, tv_mode->name))
-			return tv_mode;
-	}
-	return NULL;
+	return &tv_modes[format];
 }
 
-static const struct tv_mode *
-intel_tv_mode_find(struct intel_tv *intel_tv)
+static const struct tv_mode *intel_tv_mode_find_committed(struct drm_connector *connector)
 {
-	return intel_tv_mode_lookup(intel_tv->tv_format);
+	int format = intel_attached_tv(connector)->format;
+
+	return &tv_modes[format];
 }
 
 static enum drm_mode_status
 intel_tv_mode_valid(struct drm_connector *connector,
 		    struct drm_display_mode *mode)
 {
-	struct intel_tv *intel_tv = intel_attached_tv(connector);
-	const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
+	const struct tv_mode *tv_mode = intel_tv_mode_find_committed(connector);
 	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
 
 	if (mode->clock > max_dotclk)
@@ -925,8 +927,7 @@ intel_tv_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_state *pipe_config,
 			struct drm_connector_state *conn_state)
 {
-	struct intel_tv *intel_tv = enc_to_tv(encoder);
-	const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
+	const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state);
 
 	if (!tv_mode)
 		return false;
@@ -1032,7 +1033,7 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
 	struct intel_tv *intel_tv = enc_to_tv(encoder);
-	const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
+	const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state);
 	u32 tv_ctl;
 	u32 scctl1, scctl2, scctl3;
 	int i, j;
@@ -1135,12 +1136,12 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder,
 	else
 		ysize = 2*tv_mode->nbr_end + 1;
 
-	xpos += intel_tv->margin[TV_MARGIN_LEFT];
-	ypos += intel_tv->margin[TV_MARGIN_TOP];
-	xsize -= (intel_tv->margin[TV_MARGIN_LEFT] +
-		  intel_tv->margin[TV_MARGIN_RIGHT]);
-	ysize -= (intel_tv->margin[TV_MARGIN_TOP] +
-		  intel_tv->margin[TV_MARGIN_BOTTOM]);
+	xpos += conn_state->tv.margins.left;
+	ypos += conn_state->tv.margins.top;
+	xsize -= (conn_state->tv.margins.left +
+		  conn_state->tv.margins.right);
+	ysize -= (conn_state->tv.margins.top +
+		  conn_state->tv.margins.bottom);
 	I915_WRITE(TV_WIN_POS, (xpos<<16)|ypos);
 	I915_WRITE(TV_WIN_SIZE, (xsize<<16)|ysize);
 
@@ -1288,7 +1289,7 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
 static void intel_tv_find_better_format(struct drm_connector *connector)
 {
 	struct intel_tv *intel_tv = intel_attached_tv(connector);
-	const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
+	const struct tv_mode *tv_mode = intel_tv_mode_find(connector->state);
 	int i;
 
 	if ((intel_tv->type == DRM_MODE_CONNECTOR_Component) ==
@@ -1304,9 +1305,8 @@ static void intel_tv_find_better_format(struct drm_connector *connector)
 			break;
 	}
 
-	intel_tv->tv_format = tv_mode->name;
-	drm_object_property_set_value(&connector->base,
-		connector->dev->mode_config.tv_mode_property, i);
+	connector->state->tv.mode = i;
+	intel_attached_tv(connector)->format = i;
 }
 
 /**
@@ -1344,18 +1344,17 @@ intel_tv_detect(struct drm_connector *connector, bool force)
 		} else
 			status = connector_status_unknown;
 
+		if (status == connector_status_connected) {
+			intel_tv->type = type;
+			intel_tv_find_better_format(connector);
+		}
+
 		drm_modeset_drop_locks(&ctx);
 		drm_modeset_acquire_fini(&ctx);
-	} else
-		return connector->status;
 
-	if (status != connector_status_connected)
 		return status;
-
-	intel_tv->type = type;
-	intel_tv_find_better_format(connector);
-
-	return connector_status_connected;
+	} else
+		return connector->status;
 }
 
 static const struct input_res {
@@ -1375,12 +1374,9 @@ static const struct input_res {
  * Chose preferred mode  according to line number of TV format
  */
 static void
-intel_tv_chose_preferred_modes(struct drm_connector *connector,
+intel_tv_choose_preferred_modes(const struct tv_mode *tv_mode,
 			       struct drm_display_mode *mode_ptr)
 {
-	struct intel_tv *intel_tv = intel_attached_tv(connector);
-	const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
-
 	if (tv_mode->nbr_end < 480 && mode_ptr->vdisplay == 480)
 		mode_ptr->type |= DRM_MODE_TYPE_PREFERRED;
 	else if (tv_mode->nbr_end > 480) {
@@ -1403,8 +1399,7 @@ static int
 intel_tv_get_modes(struct drm_connector *connector)
 {
 	struct drm_display_mode *mode_ptr;
-	struct intel_tv *intel_tv = intel_attached_tv(connector);
-	const struct tv_mode *tv_mode = intel_tv_mode_find(intel_tv);
+	const struct tv_mode *tv_mode = intel_tv_mode_find_committed(connector);
 	int j, count = 0;
 	u64 tmp;
 
@@ -1447,7 +1442,7 @@ intel_tv_get_modes(struct drm_connector *connector)
 		mode_ptr->clock = (int) tmp;
 
 		mode_ptr->type = DRM_MODE_TYPE_DRIVER;
-		intel_tv_chose_preferred_modes(connector, mode_ptr);
+		intel_tv_choose_preferred_modes(tv_mode, mode_ptr);
 		drm_mode_probed_add(connector, mode_ptr);
 		count++;
 	}
@@ -1462,66 +1457,13 @@ intel_tv_destroy(struct drm_connector *connector)
 	kfree(connector);
 }
 
-
-static int
-intel_tv_set_property(struct drm_connector *connector, struct drm_property *property,
-		      uint64_t val)
-{
-	struct drm_device *dev = connector->dev;
-	struct intel_tv *intel_tv = intel_attached_tv(connector);
-	struct drm_crtc *crtc = intel_tv->base.base.crtc;
-	int ret = 0;
-	bool changed = false;
-
-	ret = drm_object_property_set_value(&connector->base, property, val);
-	if (ret < 0)
-		goto out;
-
-	if (property == dev->mode_config.tv_left_margin_property &&
-		intel_tv->margin[TV_MARGIN_LEFT] != val) {
-		intel_tv->margin[TV_MARGIN_LEFT] = val;
-		changed = true;
-	} else if (property == dev->mode_config.tv_right_margin_property &&
-		intel_tv->margin[TV_MARGIN_RIGHT] != val) {
-		intel_tv->margin[TV_MARGIN_RIGHT] = val;
-		changed = true;
-	} else if (property == dev->mode_config.tv_top_margin_property &&
-		intel_tv->margin[TV_MARGIN_TOP] != val) {
-		intel_tv->margin[TV_MARGIN_TOP] = val;
-		changed = true;
-	} else if (property == dev->mode_config.tv_bottom_margin_property &&
-		intel_tv->margin[TV_MARGIN_BOTTOM] != val) {
-		intel_tv->margin[TV_MARGIN_BOTTOM] = val;
-		changed = true;
-	} else if (property == dev->mode_config.tv_mode_property) {
-		if (val >= ARRAY_SIZE(tv_modes)) {
-			ret = -EINVAL;
-			goto out;
-		}
-		if (!strcmp(intel_tv->tv_format, tv_modes[val].name))
-			goto out;
-
-		intel_tv->tv_format = tv_modes[val].name;
-		changed = true;
-	} else {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (changed && crtc)
-		intel_crtc_restore_mode(crtc);
-out:
-	return ret;
-}
-
 static const struct drm_connector_funcs intel_tv_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = intel_tv_detect,
 	.late_register = intel_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_tv_destroy,
-	.set_property = intel_tv_set_property,
-	.atomic_get_property = intel_connector_atomic_get_property,
+	.set_property = drm_atomic_helper_connector_set_property,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -1532,6 +1474,41 @@ static const struct drm_connector_helper_funcs intel_tv_connector_helper_funcs =
 	.get_modes = intel_tv_get_modes,
 };
 
+static int intel_tv_atomic_check(struct drm_encoder *encoder,
+				 struct drm_crtc_state *crtc_state,
+				 struct drm_connector_state *new_state)
+{
+	struct drm_connector_state *old_state = new_state->connector->state;
+
+	if (old_state->tv.mode != new_state->tv.mode ||
+	    old_state->tv.margins.left != new_state->tv.margins.left ||
+	    old_state->tv.margins.right != new_state->tv.margins.right ||
+	    old_state->tv.margins.top != new_state->tv.margins.top ||
+	    old_state->tv.margins.bottom != new_state->tv.margins.bottom) {
+		int ret;
+
+		crtc_state->connectors_changed = true;
+
+		/*
+		 * This is called after drm_atomic_helper_check_modeset adds
+		 * affected planes/connectors, so we have to add them manually.
+		 */
+		ret = drm_atomic_add_affected_connectors(new_state->state, crtc_state->crtc);
+		if (ret)
+			return ret;
+
+		ret = drm_atomic_add_affected_planes(new_state->state, crtc_state->crtc);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct drm_encoder_helper_funcs intel_tv_enc_helper_funcs = {
+	.atomic_check = intel_tv_atomic_check,
+};
+
 static const struct drm_encoder_funcs intel_tv_enc_funcs = {
 	.destroy = intel_encoder_destroy,
 };
@@ -1547,6 +1524,7 @@ intel_tv_init(struct drm_i915_private *dev_priv)
 	u32 tv_dac_on, tv_dac_off, save_tv_dac;
 	const char *tv_format_names[ARRAY_SIZE(tv_modes)];
 	int i, initial_mode = 0;
+	struct drm_connector_state *state;
 
 	if ((I915_READ(TV_CTL) & TV_FUSE_STATE_MASK) == TV_FUSE_STATE_DISABLED)
 		return;
@@ -1592,6 +1570,7 @@ intel_tv_init(struct drm_i915_private *dev_priv)
 
 	intel_encoder = &intel_tv->base;
 	connector = &intel_connector->base;
+	state = connector->state;
 
 	/* The documentation, for the older chipsets at least, recommend
 	 * using a polling method rather than hotplug detection for TVs.
@@ -1617,6 +1596,7 @@ intel_tv_init(struct drm_i915_private *dev_priv)
 	intel_encoder->disable = intel_disable_tv;
 	intel_encoder->get_hw_state = intel_tv_get_hw_state;
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
+	intel_connector->swap_state = intel_tv_swap_state;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 
@@ -1627,14 +1607,16 @@ intel_tv_init(struct drm_i915_private *dev_priv)
 	intel_encoder->cloneable = 0;
 	intel_encoder->base.possible_crtcs = ((1 << 0) | (1 << 1));
 	intel_tv->type = DRM_MODE_CONNECTOR_Unknown;
+	drm_encoder_helper_add(&intel_encoder->base, &intel_tv_enc_helper_funcs);
 
 	/* BIOS margin values */
-	intel_tv->margin[TV_MARGIN_LEFT] = 54;
-	intel_tv->margin[TV_MARGIN_TOP] = 36;
-	intel_tv->margin[TV_MARGIN_RIGHT] = 46;
-	intel_tv->margin[TV_MARGIN_BOTTOM] = 37;
+	state->tv.margins.left = 54;
+	state->tv.margins.top = 36;
+	state->tv.margins.right = 46;
+	state->tv.margins.bottom = 37;
 
-	intel_tv->tv_format = tv_modes[initial_mode].name;
+	state->tv.mode = initial_mode;
+	intel_tv->format = initial_mode;
 
 	drm_connector_helper_add(connector, &intel_tv_connector_helper_funcs);
 	connector->interlace_allowed = false;
@@ -1648,17 +1630,17 @@ intel_tv_init(struct drm_i915_private *dev_priv)
 				      tv_format_names);
 
 	drm_object_attach_property(&connector->base, dev->mode_config.tv_mode_property,
-				   initial_mode);
+				   state->tv.mode);
 	drm_object_attach_property(&connector->base,
 				   dev->mode_config.tv_left_margin_property,
-				   intel_tv->margin[TV_MARGIN_LEFT]);
+				   state->tv.margins.left);
 	drm_object_attach_property(&connector->base,
 				   dev->mode_config.tv_top_margin_property,
-				   intel_tv->margin[TV_MARGIN_TOP]);
+				   state->tv.margins.top);
 	drm_object_attach_property(&connector->base,
 				   dev->mode_config.tv_right_margin_property,
-				   intel_tv->margin[TV_MARGIN_RIGHT]);
+				   state->tv.margins.right);
 	drm_object_attach_property(&connector->base,
 				   dev->mode_config.tv_bottom_margin_property,
-				   intel_tv->margin[TV_MARGIN_BOTTOM]);
+				   state->tv.margins.bottom);
 }
-- 
2.7.4


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2017-03-15 10:08 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-09 13:06 [PATCH 0/3] drm/i915: Convert tv/dp_mst and crt connector properties to atomic Maarten Lankhorst
2017-03-09 13:06 ` [PATCH 1/3] drm/i915: Convert intel_tv " Maarten Lankhorst
2017-03-09 17:36   ` Ville Syrjälä
2017-03-13  9:22     ` Maarten Lankhorst
2017-03-13  9:29       ` Ville Syrjälä
2017-03-13 10:38         ` Maarten Lankhorst
2017-03-13 10:55           ` Ville Syrjälä
2017-03-13 12:19             ` Maarten Lankhorst
2017-03-13 16:10             ` [PATCH 1/3] drm/i915: Convert intel_tv connector properties to atomic, v2 Maarten Lankhorst
2017-03-13 16:22               ` Ville Syrjälä
2017-03-13 16:35                 ` Maarten Lankhorst
2017-03-13 16:44                   ` Ville Syrjälä
2017-03-13 17:14                     ` Maarten Lankhorst
2017-03-13 16:44                 ` Maarten Lankhorst
2017-03-14 18:36               ` Ville Syrjälä
2017-03-15  8:32                 ` Maarten Lankhorst
2017-03-15 10:08                 ` Maarten Lankhorst [this message]
2017-03-16 10:48                   ` [PATCH 1/3] drm/i915: Convert intel_tv connector properties to atomic, v3 Daniel Vetter
2017-03-16 12:05                     ` Maarten Lankhorst
2017-03-16 14:56                       ` Daniel Vetter
2017-03-16 15:27                         ` Maarten Lankhorst
2017-03-16 15:43                           ` Daniel Vetter
2017-03-09 13:06 ` [PATCH 2/3] drm/i915: Convert intel_dp_mst connector properties to atomic Maarten Lankhorst
2017-03-09 13:06 ` [PATCH 3/3] drm/i915: Convert intel_crt " Maarten Lankhorst
2017-03-09 16:23 ` ✓ Fi.CI.BAT: success for drm/i915: Convert tv/dp_mst and crt " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21a8348b-aca9-3c19-3a3d-9ed766053eb9@linux.intel.com \
    --to=maarten.lankhorst@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.