[PATCH v3 00/20] Remove depencies on staged config for atomic transition

[PATCH v3 00/20] Remove depencies on staged config for atomic transition

[Ander Conselvan de Oliveira]

Version 3 of the series with comments from Chandra addressed. I'm sending the
whole series again so it goes through another round of PRTS testing.

Thanks,
Ander

Ander Conselvan de Oliveira (19):
  drm/i915: Add intel_atomic_get_crtc_state() helper function
  drm/i915: Pass acquire ctx also to intel_release_load_detect_pipe()
  drm/i915: Allocate a drm_atomic_state for the legacy modeset code
  drm/i915: Allocate a crtc_state also when the crtc is being disabled
  drm/i915: Update dummy connector atomic state with current config
  drm/i915: Implement connector state duplication
  drm/i915: Copy the staged connector config to the legacy atomic state
  drm/i915: Don't use encoder->new_crtc in intel_modeset_pipe_config()
  drm/i915: Don't use encoder->new_crtc in compute_baseline_pipe_bpp()
  drm/i915: Don't depend on encoder->new_crtc in
    intel_dp_compute_config()
  drm/i915: Don't depend on encoder->new_crtc in
    intel_hdmi_compute_config
  drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder()
  drm/i915: Don't use staged config in intel_dp_mst_compute_config()
  drm/i915: Don't use encoder->new_crtc in intel_lvds_compute_config()
  drm/i915: Pass an atomic state to modeset_global_resources() functions
  drm/i915: Check lane sharing between pipes B & C using atomic state
  drm/i915: Convert intel_pipe_will_have_type() to using atomic state
  drm/i915: Don't look at staged config crtc when changing DRRS state
  drm/i915: Remove usage of encoder->new_crtc from clock computations

Daniel Vetter (1):
  drm/i915: Add module param to test the load detect code

 drivers/gpu/drm/i915/i915_drv.h      |   5 +-
 drivers/gpu/drm/i915/i915_params.c   |   8 +-
 drivers/gpu/drm/i915/intel_crt.c     |   9 +-
 drivers/gpu/drm/i915/intel_ddi.c     |  24 +-
 drivers/gpu/drm/i915/intel_display.c | 566 +++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_dp.c      |   5 +-
 drivers/gpu/drm/i915/intel_dp_mst.c  |  18 +-
 drivers/gpu/drm/i915/intel_drv.h     |  16 +-
 drivers/gpu/drm/i915/intel_dsi.c     |   1 +
 drivers/gpu/drm/i915/intel_dvo.c     |   1 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  22 +-
 drivers/gpu/drm/i915/intel_lvds.c    |   3 +-
 drivers/gpu/drm/i915/intel_sdvo.c    |   1 +
 drivers/gpu/drm/i915/intel_tv.c      |   3 +-
 14 files changed, 523 insertions(+), 159 deletions(-)

-- 
2.1.0

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

[PATCH 01/20] drm/i915: Add intel_atomic_get_crtc_state() helper function

[Ander Conselvan de Oliveira]

The pattern of getting the crtc state with drm_atomic_get_crtc_state()
and then converting it to intel_crtc_state will repeat quite often in
the following patches, so add a helper function to save some typing.

v2: Fix upcasting so that crtc_state base field could be moved. (Daniel)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c77128c..5bae4aa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -35,6 +35,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_dp_mst_helper.h>
 #include <drm/drm_rect.h>
+#include <drm/drm_atomic.h>
 
 #define DIV_ROUND_CLOSEST_ULL(ll, d)	\
 ({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
@@ -564,6 +565,7 @@ struct cxsr_latency {
 };
 
 #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
+#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, base)
 #define to_intel_connector(x) container_of(x, struct intel_connector, base)
 #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
 #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
@@ -1282,6 +1284,17 @@ int intel_connector_atomic_get_property(struct drm_connector *connector,
 struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
 void intel_crtc_destroy_state(struct drm_crtc *crtc,
 			       struct drm_crtc_state *state);
+static inline struct intel_crtc_state *
+intel_atomic_get_crtc_state(struct drm_atomic_state *state,
+			    struct intel_crtc *crtc)
+{
+	struct drm_crtc_state *crtc_state;
+	crtc_state = drm_atomic_get_crtc_state(state, &crtc->base);
+	if (IS_ERR(crtc_state))
+		return ERR_PTR(PTR_ERR(crtc_state));
+
+	return to_intel_crtc_state(crtc_state);
+}
 
 /* intel_atomic_plane.c */
 struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
-- 
2.1.0

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

[PATCH 02/20] drm/i915: Pass acquire ctx also to intel_release_load_detect_pipe()

[Ander Conselvan de Oliveira]

For now this is not necessary since intel_set_mode() doesn't acquire any
new locks. However, once that function is converted to atomic, that will
change, since we'll pass an atomic state to it, and that needs to have
the right acquire context set.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     | 2 +-
 drivers/gpu/drm/i915/intel_display.c | 5 +++--
 drivers/gpu/drm/i915/intel_drv.h     | 3 ++-
 drivers/gpu/drm/i915/intel_tv.c      | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index e66e17a..974534e 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -708,7 +708,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 			status = connector_status_connected;
 		else
 			status = intel_crt_load_detect(crt);
-		intel_release_load_detect_pipe(connector, &tmp);
+		intel_release_load_detect_pipe(connector, &tmp, &ctx);
 	} else
 		status = connector_status_unknown;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 90b460c..8458bf5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8933,7 +8933,8 @@ fail_unlock:
 }
 
 void intel_release_load_detect_pipe(struct drm_connector *connector,
-				    struct intel_load_detect_pipe *old)
+				    struct intel_load_detect_pipe *old,
+				    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct intel_encoder *intel_encoder =
 		intel_attached_encoder(connector);
@@ -13479,7 +13480,7 @@ static void intel_enable_pipe_a(struct drm_device *dev)
 		return;
 
 	if (intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, ctx))
-		intel_release_load_detect_pipe(crt, &load_detect_temp);
+		intel_release_load_detect_pipe(crt, &load_detect_temp, ctx);
 }
 
 static bool
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5bae4aa..ec99046 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -958,7 +958,8 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 				struct intel_load_detect_pipe *old,
 				struct drm_modeset_acquire_ctx *ctx);
 void intel_release_load_detect_pipe(struct drm_connector *connector,
-				    struct intel_load_detect_pipe *old);
+				    struct intel_load_detect_pipe *old,
+				    struct drm_modeset_acquire_ctx *ctx);
 int intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 			       struct drm_framebuffer *fb,
 			       struct intel_engine_cs *pipelined);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 892d23c..4e6684e 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1332,7 +1332,7 @@ intel_tv_detect(struct drm_connector *connector, bool force)
 
 		if (intel_get_load_detect_pipe(connector, &mode, &tmp, &ctx)) {
 			type = intel_tv_detect_type(intel_tv, connector);
-			intel_release_load_detect_pipe(connector, &tmp);
+			intel_release_load_detect_pipe(connector, &tmp, &ctx);
 			status = type < 0 ?
 				connector_status_disconnected :
 				connector_status_connected;
-- 
2.1.0

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

[PATCH 03/20] drm/i915: Allocate a drm_atomic_state for the legacy modeset code

[Ander Conselvan de Oliveira]

For the atomic conversion, the mode set paths need to be changed to rely
on an atomic state instead of using the staged config. By using an
atomic state for the legacy code, we will be able to convert the code
base in small chunks.

v2: Squash patch that adds stat argument to intel_set_mode(). (Ander)
    Make every caller of intel_set_mode() allocate state. (Daniel)
    Call drm_atomic_state_clear() in set config's error path. (Daniel)

v3: Copy staged config to atomic state in force restore path. (Ander)

v4: Don't update ->new_config for disabled pipes in __intel_set_mode(),
    since it is expected to be NULL in that case. (Ander)

v5: Don't change return type of intel_modeset_pipe_config(). (Chandra)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 186 +++++++++++++++++++++++++++++------
 1 file changed, 156 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8458bf5..b848646 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -83,7 +83,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config);
 
 static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
-			  int x, int y, struct drm_framebuffer *old_fb);
+			  int x, int y, struct drm_framebuffer *old_fb,
+			  struct drm_atomic_state *state);
 static int intel_framebuffer_init(struct drm_device *dev,
 				  struct intel_framebuffer *ifb,
 				  struct drm_mode_fb_cmd2 *mode_cmd,
@@ -8802,6 +8803,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct drm_device *dev = encoder->dev;
 	struct drm_framebuffer *fb;
 	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_atomic_state *state = NULL;
 	int ret, i = -1;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
@@ -8883,6 +8885,12 @@ retry:
 	old->load_detect_temp = true;
 	old->release_fb = NULL;
 
+	state = drm_atomic_state_alloc(dev);
+	if (!state)
+		return false;
+
+	state->acquire_ctx = ctx;
+
 	if (!mode)
 		mode = &load_detect_mode;
 
@@ -8905,7 +8913,7 @@ retry:
 		goto fail;
 	}
 
-	if (intel_set_mode(crtc, mode, 0, 0, fb)) {
+	if (intel_set_mode(crtc, mode, 0, 0, fb, state)) {
 		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
 		if (old->release_fb)
 			old->release_fb->funcs->destroy(old->release_fb);
@@ -8924,6 +8932,11 @@ retry:
 	else
 		intel_crtc->new_config = NULL;
 fail_unlock:
+	if (state) {
+		drm_atomic_state_free(state);
+		state = NULL;
+	}
+
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(ctx);
 		goto retry;
@@ -8936,22 +8949,34 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 				    struct intel_load_detect_pipe *old,
 				    struct drm_modeset_acquire_ctx *ctx)
 {
+	struct drm_device *dev = connector->dev;
 	struct intel_encoder *intel_encoder =
 		intel_attached_encoder(connector);
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = encoder->crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_atomic_state *state;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 		      connector->base.id, connector->name,
 		      encoder->base.id, encoder->name);
 
 	if (old->load_detect_temp) {
+		state = drm_atomic_state_alloc(dev);
+		if (!state) {
+			DRM_DEBUG_KMS("can't release load detect pipe\n");
+			return;
+		}
+
+		state->acquire_ctx = ctx;
+
 		to_intel_connector(connector)->new_encoder = NULL;
 		intel_encoder->new_crtc = NULL;
 		intel_crtc->new_enabled = false;
 		intel_crtc->new_config = NULL;
-		intel_set_mode(crtc, NULL, 0, 0, NULL);
+		intel_set_mode(crtc, NULL, 0, 0, NULL, state);
+
+		drm_atomic_state_free(state);
 
 		if (old->release_fb) {
 			drm_framebuffer_unregister_private(old->release_fb);
@@ -10345,10 +10370,22 @@ static bool check_digital_port_conflicts(struct drm_device *dev)
 	return true;
 }
 
+static void
+clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
+{
+	struct drm_crtc_state tmp_state;
+
+	/* Clear only the intel specific part of the crtc state */
+	tmp_state = crtc_state->base;
+	memset(crtc_state, 0, sizeof *crtc_state);
+	crtc_state->base = tmp_state;
+}
+
 static struct intel_crtc_state *
 intel_modeset_pipe_config(struct drm_crtc *crtc,
 			  struct drm_framebuffer *fb,
-			  struct drm_display_mode *mode)
+			  struct drm_display_mode *mode,
+			  struct drm_atomic_state *state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct intel_encoder *encoder;
@@ -10366,9 +10403,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 		return ERR_PTR(-EINVAL);
 	}
 
-	pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
-	if (!pipe_config)
-		return ERR_PTR(-ENOMEM);
+	pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
+	if (IS_ERR(pipe_config))
+		return pipe_config;
+
+	clear_intel_crtc_state(pipe_config);
 
 	pipe_config->base.crtc = crtc;
 	drm_mode_copy(&pipe_config->base.adjusted_mode, mode);
@@ -10465,7 +10504,6 @@ encoder_retry:
 
 	return pipe_config;
 fail:
-	kfree(pipe_config);
 	return ERR_PTR(ret);
 }
 
@@ -11144,17 +11182,19 @@ static struct intel_crtc_state *
 intel_modeset_compute_config(struct drm_crtc *crtc,
 			     struct drm_display_mode *mode,
 			     struct drm_framebuffer *fb,
+			     struct drm_atomic_state *state,
 			     unsigned *modeset_pipes,
 			     unsigned *prepare_pipes,
 			     unsigned *disable_pipes)
 {
 	struct intel_crtc_state *pipe_config = NULL;
+	int ret = 0;
 
 	intel_modeset_affected_pipes(crtc, modeset_pipes,
 				     prepare_pipes, disable_pipes);
 
 	if ((*modeset_pipes) == 0)
-		goto out;
+		return NULL;
 
 	/*
 	 * Note this needs changes when we start tracking multiple modes
@@ -11162,14 +11202,13 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	 * (i.e. one pipe_config for each crtc) rather than just the one
 	 * for this crtc.
 	 */
-	pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
-	if (IS_ERR(pipe_config)) {
-		goto out;
-	}
+	pipe_config = intel_modeset_pipe_config(crtc, fb, mode, state);
+	if (IS_ERR(pipe_config))
+		return pipe_config;
+
 	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
 			       "[modeset]");
 
-out:
 	return pipe_config;
 }
 
@@ -11214,6 +11253,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_display_mode *saved_mode;
+	struct intel_crtc_state *crtc_state_copy = NULL;
 	struct intel_crtc *intel_crtc;
 	int ret = 0;
 
@@ -11221,6 +11261,12 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	if (!saved_mode)
 		return -ENOMEM;
 
+	crtc_state_copy = kmalloc(sizeof(*crtc_state_copy), GFP_KERNEL);
+	if (!crtc_state_copy) {
+		ret = -ENOMEM;
+		goto done;
+	}
+
 	*saved_mode = crtc->mode;
 
 	if (modeset_pipes)
@@ -11307,6 +11353,22 @@ done:
 	if (ret && crtc->state->enable)
 		crtc->mode = *saved_mode;
 
+	if (ret == 0 && pipe_config) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+		/* The pipe_config will be freed with the atomic state, so
+		 * make a copy. */
+		memcpy(crtc_state_copy, intel_crtc->config,
+		       sizeof *crtc_state_copy);
+		intel_crtc->config = crtc_state_copy;
+		intel_crtc->base.state = &crtc_state_copy->base;
+
+		if (modeset_pipes)
+			intel_crtc->new_config = intel_crtc->config;
+	} else {
+		kfree(crtc_state_copy);
+	}
+
 	kfree(saved_mode);
 	return ret;
 }
@@ -11332,27 +11394,81 @@ static int intel_set_mode_pipes(struct drm_crtc *crtc,
 
 static int intel_set_mode(struct drm_crtc *crtc,
 			  struct drm_display_mode *mode,
-			  int x, int y, struct drm_framebuffer *fb)
+			  int x, int y, struct drm_framebuffer *fb,
+			  struct drm_atomic_state *state)
 {
 	struct intel_crtc_state *pipe_config;
 	unsigned modeset_pipes, prepare_pipes, disable_pipes;
+	int ret = 0;
 
-	pipe_config = intel_modeset_compute_config(crtc, mode, fb,
+	pipe_config = intel_modeset_compute_config(crtc, mode, fb, state,
 						   &modeset_pipes,
 						   &prepare_pipes,
 						   &disable_pipes);
 
-	if (IS_ERR(pipe_config))
-		return PTR_ERR(pipe_config);
+	if (IS_ERR(pipe_config)) {
+		ret = PTR_ERR(pipe_config);
+		goto out;
+	}
+
+	ret = intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
+				   modeset_pipes, prepare_pipes,
+				   disable_pipes);
+	if (ret)
+		goto out;
 
-	return intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
-				    modeset_pipes, prepare_pipes,
-				    disable_pipes);
+out:
+	return ret;
 }
 
 void intel_crtc_restore_mode(struct drm_crtc *crtc)
 {
-	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb);
+	struct drm_device *dev = crtc->dev;
+	struct drm_atomic_state *state;
+	struct intel_encoder *encoder;
+	struct intel_connector *connector;
+	struct drm_connector_state *connector_state;
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state) {
+		DRM_DEBUG_KMS("[CRTC:%d] mode restore failed, out of memory",
+			      crtc->base.id);
+		return;
+	}
+
+	state->acquire_ctx = dev->mode_config.acquire_ctx;
+
+	/* The force restore path in the HW readout code relies on the staged
+	 * config still keeping the user requested config while the actual
+	 * state has been overwritten by the configuration read from HW. We
+	 * need to copy the staged config to the atomic state, otherwise the
+	 * mode set will just reapply the state the HW is already in. */
+	for_each_intel_encoder(dev, encoder) {
+		if (&encoder->new_crtc->base != crtc)
+			continue;
+
+		for_each_intel_connector(dev, connector) {
+			if (connector->new_encoder != encoder)
+				continue;
+
+			connector_state = drm_atomic_get_connector_state(state, &connector->base);
+			if (IS_ERR(connector_state)) {
+				DRM_DEBUG_KMS("Failed to add [CONNECTOR:%d:%s] to state: %ld\n",
+					      connector->base.base.id,
+					      connector->base.name,
+					      PTR_ERR(connector_state));
+				continue;
+			}
+
+			connector_state->crtc = crtc;
+			connector_state->best_encoder = &encoder->base;
+		}
+	}
+
+	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb,
+		       state);
+
+	drm_atomic_state_free(state);
 }
 
 #undef for_each_intel_crtc_masked
@@ -11677,6 +11793,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 {
 	struct drm_device *dev;
 	struct drm_mode_set save_set;
+	struct drm_atomic_state *state = NULL;
 	struct intel_set_config *config;
 	struct intel_crtc_state *pipe_config;
 	unsigned modeset_pipes, prepare_pipes, disable_pipes;
@@ -11721,12 +11838,20 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	 * such cases. */
 	intel_set_config_compute_mode_changes(set, config);
 
+	state = drm_atomic_state_alloc(dev);
+	if (!state) {
+		ret = -ENOMEM;
+		goto out_config;
+	}
+
+	state->acquire_ctx = dev->mode_config.acquire_ctx;
+
 	ret = intel_modeset_stage_output_state(dev, set, config);
 	if (ret)
 		goto fail;
 
 	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
-						   set->fb,
+						   set->fb, state,
 						   &modeset_pipes,
 						   &prepare_pipes,
 						   &disable_pipes);
@@ -11746,10 +11871,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 		 */
 	}
 
-	/* set_mode will free it in the mode_changed case */
-	if (!config->mode_changed)
-		kfree(pipe_config);
-
 	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
 	if (config->mode_changed) {
@@ -11795,6 +11916,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 fail:
 		intel_set_config_restore_state(dev, config);
 
+		drm_atomic_state_clear(state);
+
 		/*
 		 * HACK: if the pipe was on, but we didn't have a framebuffer,
 		 * force the pipe off to avoid oopsing in the modeset code
@@ -11807,11 +11930,15 @@ fail:
 		/* Try to restore the config */
 		if (config->mode_changed &&
 		    intel_set_mode(save_set.crtc, save_set.mode,
-				   save_set.x, save_set.y, save_set.fb))
+				   save_set.x, save_set.y, save_set.fb,
+				   state))
 			DRM_ERROR("failed to restore config after modeset failure\n");
 	}
 
 out_config:
+	if (state)
+		drm_atomic_state_free(state);
+
 	intel_set_config_free(config);
 	return ret;
 }
@@ -13852,8 +13979,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 			struct drm_crtc *crtc =
 				dev_priv->pipe_to_crtc_mapping[pipe];
 
-			intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
-				       crtc->primary->fb);
+			intel_crtc_restore_mode(crtc);
 		}
 	} else {
 		intel_modeset_update_staged_output_state(dev);
-- 
2.1.0

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

[PATCH 04/20] drm/i915: Allocate a crtc_state also when the crtc is being disabled

[Ander Conselvan de Oliveira]

For consistency, allocate a new crtc_state for a crtc that is being
disabled. Previously only the enabled value of the current state would
change.

v2: Rebase on v5 of previous patch. (Ander)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b848646..b847862 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11187,14 +11187,21 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 			     unsigned *prepare_pipes,
 			     unsigned *disable_pipes)
 {
+	struct drm_device *dev = crtc->dev;
 	struct intel_crtc_state *pipe_config = NULL;
+	struct intel_crtc *intel_crtc;
 	int ret = 0;
 
 	intel_modeset_affected_pipes(crtc, modeset_pipes,
 				     prepare_pipes, disable_pipes);
 
-	if ((*modeset_pipes) == 0)
-		return NULL;
+	for_each_intel_crtc_masked(dev, *disable_pipes, intel_crtc) {
+		pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+		if (IS_ERR(pipe_config))
+			return pipe_config;
+
+		pipe_config->base.enable = false;
+	}
 
 	/*
 	 * Note this needs changes when we start tracking multiple modes
@@ -11202,14 +11209,21 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	 * (i.e. one pipe_config for each crtc) rather than just the one
 	 * for this crtc.
 	 */
-	pipe_config = intel_modeset_pipe_config(crtc, fb, mode, state);
-	if (IS_ERR(pipe_config))
-		return pipe_config;
+	for_each_intel_crtc_masked(dev, *modeset_pipes, intel_crtc) {
+		/* FIXME: For now we still expect modeset_pipes has at most
+		 * one bit set. */
+		if (WARN_ON(&intel_crtc->base != crtc))
+			continue;
 
-	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
-			       "[modeset]");
+		pipe_config = intel_modeset_pipe_config(crtc, fb, mode, state);
+		if (IS_ERR(pipe_config))
+			return pipe_config;
 
-	return pipe_config;
+		intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+				       "[modeset]");
+	}
+
+	return intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));;
 }
 
 static int __intel_set_mode_setup_plls(struct drm_device *dev,
-- 
2.1.0

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

[PATCH 05/20] drm/i915: Update dummy connector atomic state with current config

[Ander Conselvan de Oliveira]

Keep that state updated so that we can write code that depends on it on
the follow up patches.

v2: Fix BUG due to stale connector_state->crtc value. (Chandra)

v3: Update comment about dummy state connectors. (Chandra)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 45 ++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b847862..0ed7822 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10113,6 +10113,27 @@ static void intel_modeset_update_staged_output_state(struct drm_device *dev)
 	}
 }
 
+/* Transitional helper to copy current connector/encoder state to
+ * connector->state. This is needed so that code that is partially
+ * converted to atomic does the right thing.
+ */
+static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
+{
+	struct intel_connector *connector;
+
+	for_each_intel_connector(dev, connector) {
+		if (connector->base.encoder) {
+			connector->base.state->best_encoder =
+				connector->base.encoder;
+			connector->base.state->crtc =
+				connector->base.encoder->crtc;
+		} else {
+			connector->base.state->best_encoder = NULL;
+			connector->base.state->crtc = NULL;
+		}
+	}
+}
+
 /**
  * intel_modeset_commit_output_state
  *
@@ -10136,6 +10157,8 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
 		crtc->base.state->enable = crtc->new_enabled;
 		crtc->base.enabled = crtc->new_enabled;
 	}
+
+	intel_modeset_update_connector_atomic_state(dev);
 }
 
 static void
@@ -12900,19 +12923,21 @@ static void intel_setup_outputs(struct drm_device *dev)
 	 * testing/debug of the plane operations (and only when a specific
 	 * kernel module option is given), that shouldn't really matter.
 	 *
+	 * We are also relying on these states to convert the legacy mode set
+	 * to use a drm_atomic_state struct. The states are kept consistent
+	 * with actual state, so that it is safe to rely on that instead of
+	 * the staged config.
+	 *
 	 * Once atomic support for crtc's + connectors lands, this loop should
 	 * be removed since we'll be setting up real connector state, which
 	 * will contain Intel-specific properties.
 	 */
-	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
-		list_for_each_entry(connector,
-				    &dev->mode_config.connector_list,
-				    head) {
-			if (!WARN_ON(connector->state)) {
-				connector->state =
-					kzalloc(sizeof(*connector->state),
-						GFP_KERNEL);
-			}
+	list_for_each_entry(connector,
+			    &dev->mode_config.connector_list,
+			    head) {
+		if (!WARN_ON(connector->state)) {
+			connector->state = kzalloc(sizeof(*connector->state),
+						   GFP_KERNEL);
 		}
 	}
 
@@ -13965,6 +13990,8 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 				       "[setup_hw_state]");
 	}
 
+	intel_modeset_update_connector_atomic_state(dev);
+
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
 		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
 
-- 
2.1.0

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

[PATCH 06/20] drm/i915: Implement connector state duplication

[Ander Conselvan de Oliveira]

So that we can add connector states to the drm_atomic_state used in the
legacy modeset.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c    | 1 +
 drivers/gpu/drm/i915/intel_dp.c     | 1 +
 drivers/gpu/drm/i915/intel_dp_mst.c | 1 +
 drivers/gpu/drm/i915/intel_dsi.c    | 1 +
 drivers/gpu/drm/i915/intel_dvo.c    | 1 +
 drivers/gpu/drm/i915/intel_hdmi.c   | 1 +
 drivers/gpu/drm/i915/intel_lvds.c   | 1 +
 drivers/gpu/drm/i915/intel_sdvo.c   | 1 +
 drivers/gpu/drm/i915/intel_tv.c     | 1 +
 9 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 974534e..573aaff 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -794,6 +794,7 @@ static const struct drm_connector_funcs intel_crt_connector_funcs = {
 	.destroy = intel_crt_destroy,
 	.set_property = intel_crt_set_property,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_get_property = intel_connector_atomic_get_property,
 };
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ca60060..550db30 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4548,6 +4548,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
 	.atomic_get_property = intel_connector_atomic_get_property,
 	.destroy = intel_dp_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
 static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index be12492..5c06a06 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -317,6 +317,7 @@ static const struct drm_connector_funcs intel_dp_mst_connector_funcs = {
 	.atomic_get_property = intel_connector_atomic_get_property,
 	.destroy = intel_dp_mst_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
 static int intel_dp_mst_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index c8c8b24..572251e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -975,6 +975,7 @@ static const struct drm_connector_funcs intel_dsi_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_get_property = intel_connector_atomic_get_property,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
 void intel_dsi_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index d857951..4ccd6c3 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -393,6 +393,7 @@ static const struct drm_connector_funcs intel_dvo_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_get_property = intel_connector_atomic_get_property,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
 static const struct drm_connector_helper_funcs intel_dvo_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 995c5b2..b13a8be 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1618,6 +1618,7 @@ static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
 	.atomic_get_property = intel_connector_atomic_get_property,
 	.destroy = intel_hdmi_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
 static const struct drm_connector_helper_funcs intel_hdmi_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 24e8730..2b008b02 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -535,6 +535,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
 	.atomic_get_property = intel_connector_atomic_get_property,
 	.destroy = intel_lvds_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
 static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 9e554c2..f5b7e1e 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2194,6 +2194,7 @@ static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
 	.atomic_get_property = intel_connector_atomic_get_property,
 	.destroy = intel_sdvo_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 };
 
 static const struct drm_connector_helper_funcs intel_sdvo_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 4e6684e..bc1d9d7 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1516,6 +1516,7 @@ static const struct drm_connector_funcs intel_tv_connector_funcs = {
 	.atomic_get_property = intel_connector_atomic_get_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,
 };
 
 static const struct drm_connector_helper_funcs intel_tv_connector_helper_funcs = {
-- 
2.1.0

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

[PATCH 07/20] drm/i915: Copy the staged connector config to the legacy atomic state

[Ander Conselvan de Oliveira]

With this in place, we can start converting pieces of the modeset code
to look at the connector atomic state instead of the staged config.

v2: Handle the load detect staged config changes too. (Ander)
    Remove unnecessary blank line. (Daniel)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 52 +++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0ed7822..b76726f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8804,6 +8804,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct drm_framebuffer *fb;
 	struct drm_mode_config *config = &dev->mode_config;
 	struct drm_atomic_state *state = NULL;
+	struct drm_connector_state *connector_state;
 	int ret, i = -1;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
@@ -8891,6 +8892,15 @@ retry:
 
 	state->acquire_ctx = ctx;
 
+	connector_state = drm_atomic_get_connector_state(state, connector);
+	if (IS_ERR(connector_state)) {
+		ret = PTR_ERR(connector_state);
+		goto fail;
+	}
+
+	connector_state->crtc = crtc;
+	connector_state->best_encoder = &intel_encoder->base;
+
 	if (!mode)
 		mode = &load_detect_mode;
 
@@ -8956,6 +8966,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 	struct drm_crtc *crtc = encoder->crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_atomic_state *state;
+	struct drm_connector_state *connector_state;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 		      connector->base.id, connector->name,
@@ -8963,17 +8974,23 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 
 	if (old->load_detect_temp) {
 		state = drm_atomic_state_alloc(dev);
-		if (!state) {
-			DRM_DEBUG_KMS("can't release load detect pipe\n");
-			return;
-		}
+		if (!state)
+			goto fail;
 
 		state->acquire_ctx = ctx;
 
+		connector_state = drm_atomic_get_connector_state(state, connector);
+		if (IS_ERR(connector_state))
+			goto fail;
+
 		to_intel_connector(connector)->new_encoder = NULL;
 		intel_encoder->new_crtc = NULL;
 		intel_crtc->new_enabled = false;
 		intel_crtc->new_config = NULL;
+
+		connector_state->best_encoder = NULL;
+		connector_state->crtc = NULL;
+
 		intel_set_mode(crtc, NULL, 0, 0, NULL, state);
 
 		drm_atomic_state_free(state);
@@ -8989,6 +9006,11 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 	/* Switch crtc and encoder back off if necessary */
 	if (old->dpms_mode != DRM_MODE_DPMS_ON)
 		connector->funcs->dpms(connector, old->dpms_mode);
+
+	return;
+fail:
+	DRM_DEBUG_KMS("Couldn't release load detect pipe.\n");
+	drm_atomic_state_free(state);
 }
 
 static int i9xx_pll_refclk(struct drm_device *dev,
@@ -11674,9 +11696,11 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 static int
 intel_modeset_stage_output_state(struct drm_device *dev,
 				 struct drm_mode_set *set,
-				 struct intel_set_config *config)
+				 struct intel_set_config *config,
+				 struct drm_atomic_state *state)
 {
 	struct intel_connector *connector;
+	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
 	struct intel_crtc *crtc;
 	int ro;
@@ -11740,6 +11764,14 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 		}
 		connector->new_encoder->new_crtc = to_intel_crtc(new_crtc);
 
+		connector_state =
+			drm_atomic_get_connector_state(state, &connector->base);
+		if (IS_ERR(connector_state))
+			return PTR_ERR(connector_state);
+
+		connector_state->crtc = new_crtc;
+		connector_state->best_encoder = &connector->new_encoder->base;
+
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
 			connector->base.base.id,
 			connector->base.name,
@@ -11772,9 +11804,15 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	}
 	/* Now we've also updated encoder->new_crtc for all encoders. */
 	for_each_intel_connector(dev, connector) {
-		if (connector->new_encoder)
+		connector_state =
+			drm_atomic_get_connector_state(state, &connector->base);
+
+		if (connector->new_encoder) {
 			if (connector->new_encoder != connector->encoder)
 				connector->encoder = connector->new_encoder;
+		} else {
+			connector_state->crtc = NULL;
+		}
 	}
 	for_each_intel_crtc(dev, crtc) {
 		crtc->new_enabled = false;
@@ -11883,7 +11921,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 
 	state->acquire_ctx = dev->mode_config.acquire_ctx;
 
-	ret = intel_modeset_stage_output_state(dev, set, config);
+	ret = intel_modeset_stage_output_state(dev, set, config, state);
 	if (ret)
 		goto fail;
 
-- 
2.1.0

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

[PATCH 08/20] drm/i915: Don't use encoder->new_crtc in intel_modeset_pipe_config()

[Ander Conselvan de Oliveira]

Move towards atomic by using the legacy modeset's drm_atomic_state
instead.

v2: Move call to drm_atomic_add_affected_connectors() to
    intel_modeset_compute_config(). (Daniel)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b76726f..7851d6c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10434,8 +10434,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct intel_encoder *encoder;
+	struct intel_connector *connector;
+	struct drm_connector_state *connector_state;
 	struct intel_crtc_state *pipe_config;
 	int plane_bpp, ret = -EINVAL;
+	int i;
 	bool retry = true;
 
 	if (!check_encoder_cloning(to_intel_crtc(crtc))) {
@@ -10509,11 +10512,17 @@ encoder_retry:
 	 * adjust it according to limitations or connector properties, and also
 	 * a chance to reject the mode entirely.
 	 */
-	for_each_intel_encoder(dev, encoder) {
+	for (i = 0; i < state->num_connector; i++) {
+		connector = to_intel_connector(state->connectors[i]);
+		if (!connector)
+			continue;
 
-		if (&encoder->new_crtc->base != crtc)
+		connector_state = state->connector_states[i];
+		if (connector_state->crtc != crtc)
 			continue;
 
+		encoder = to_intel_encoder(connector_state->best_encoder);
+
 		if (!(encoder->compute_config(encoder, pipe_config))) {
 			DRM_DEBUG_KMS("Encoder config failure\n");
 			goto fail;
@@ -11237,6 +11246,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc;
 	int ret = 0;
 
+	ret = drm_atomic_add_affected_connectors(state, crtc);
+	if (ret)
+		return ERR_PTR(ret);
+
 	intel_modeset_affected_pipes(crtc, modeset_pipes,
 				     prepare_pipes, disable_pipes);
 
-- 
2.1.0

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

[PATCH 09/20] drm/i915: Don't use encoder->new_crtc in compute_baseline_pipe_bpp()

[Ander Conselvan de Oliveira]

Move towards atomic by using the legacy modeset's drm_atomic_state
instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7851d6c..895cf67 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10215,8 +10215,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 			  struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev = crtc->base.dev;
+	struct drm_atomic_state *state;
 	struct intel_connector *connector;
-	int bpp;
+	int bpp, i;
 
 	switch (fb->pixel_format) {
 	case DRM_FORMAT_C8:
@@ -10256,10 +10257,13 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 
 	pipe_config->pipe_bpp = bpp;
 
+	state = pipe_config->base.state;
+
 	/* Clamp display bpp to EDID value */
-	for_each_intel_connector(dev, connector) {
-		if (!connector->new_encoder ||
-		    connector->new_encoder->new_crtc != crtc)
+	for (i = 0; i < state->num_connector; i++) {
+		connector = to_intel_connector(state->connectors[i]);
+		if (!connector ||
+		    state->connector_states[i]->crtc != &crtc->base)
 			continue;
 
 		connected_sink_compute_bpp(connector, pipe_config);
-- 
2.1.0

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

[PATCH 10/20] drm/i915: Don't depend on encoder->new_crtc in intel_dp_compute_config()

[Ander Conselvan de Oliveira]

Move towards atomic by using the legacy modeset's drm_atomic_state
instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 550db30..0c61aed 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1268,7 +1268,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	enum port port = dp_to_dig_port(intel_dp)->port;
-	struct intel_crtc *intel_crtc = encoder->new_crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	int lane_count, clock;
 	int min_lane_count = 1;
-- 
2.1.0

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

[PATCH 11/20] drm/i915: Don't depend on encoder->new_crtc in intel_hdmi_compute_config

[Ander Conselvan de Oliveira]

Move towards atomic by using the legacy modeset's drm_atomic_state
instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index b13a8be..cacbafd 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -951,19 +951,30 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	return MODE_OK;
 }
 
-static bool hdmi_12bpc_possible(struct intel_crtc *crtc)
+static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
+	struct drm_atomic_state *state;
 	struct intel_encoder *encoder;
+	struct drm_connector_state *connector_state;
 	int count = 0, count_hdmi = 0;
+	int i;
 
 	if (HAS_GMCH_DISPLAY(dev))
 		return false;
 
-	for_each_intel_encoder(dev, encoder) {
-		if (encoder->new_crtc != crtc)
+	state = crtc_state->base.state;
+
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i])
 			continue;
 
+		connector_state = state->connector_states[i];
+		if (connector_state->crtc != crtc_state->base.crtc)
+			continue;
+
+		encoder = to_intel_encoder(connector_state->best_encoder);
+
 		count_hdmi += encoder->type == INTEL_OUTPUT_HDMI;
 		count++;
 	}
@@ -1020,7 +1031,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	 */
 	if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink &&
 	    clock_12bpc <= portclock_limit &&
-	    hdmi_12bpc_possible(encoder->new_crtc)) {
+	    hdmi_12bpc_possible(pipe_config)) {
 		DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
 		desired_bpp = 12*3;
 
-- 
2.1.0

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

[PATCH 12/20] drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder()

[Ander Conselvan de Oliveira]

Instead of using connector->new_encoder, get the same information from
the pipe_config, thus making the function ready for the atomic
conversion.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 8aee7d7..47b9307 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -492,17 +492,23 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
 }
 
 static struct intel_encoder *
-intel_ddi_get_crtc_new_encoder(struct intel_crtc *crtc)
+intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct intel_encoder *intel_encoder, *ret = NULL;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct intel_encoder *ret = NULL;
+	struct drm_atomic_state *state;
 	int num_encoders = 0;
+	int i;
 
-	for_each_intel_encoder(dev, intel_encoder) {
-		if (intel_encoder->new_crtc == crtc) {
-			ret = intel_encoder;
-			num_encoders++;
-		}
+	state = crtc_state->base.state;
+
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i] ||
+		    state->connector_states[i]->crtc != crtc_state->base.crtc)
+			continue;
+
+		ret = to_intel_encoder(state->connector_states[i]->best_encoder);
+		num_encoders++;
 	}
 
 	WARN(num_encoders != 1, "%d encoders on crtc for pipe %c\n", num_encoders,
@@ -1216,7 +1222,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct intel_encoder *intel_encoder =
-		intel_ddi_get_crtc_new_encoder(intel_crtc);
+		intel_ddi_get_crtc_new_encoder(crtc_state);
 	int clock = crtc_state->port_clock;
 
 	if (IS_SKYLAKE(dev))
-- 
2.1.0

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

[PATCH 13/20] drm/i915: Don't use staged config in intel_dp_mst_compute_config()

[Ander Conselvan de Oliveira]

Move towards atomic by using the legacy modeset's drm_atomic_state
instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_dp_mst.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 5c06a06..b132fe6 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -36,11 +36,11 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
-	struct drm_device *dev = encoder->base.dev;
-	int bpp;
+	struct drm_atomic_state *state;
+	int bpp, i;
 	int lane_count, slots;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	struct intel_connector *found = NULL, *intel_connector;
+	struct intel_connector *found = NULL;
 	int mst_pbn;
 
 	pipe_config->dp_encoder_is_mst = true;
@@ -58,9 +58,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	pipe_config->pipe_bpp = 24;
 	pipe_config->port_clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
 
-	for_each_intel_connector(dev, intel_connector) {
-		if (intel_connector->new_encoder == encoder) {
-			found = intel_connector;
+	state = pipe_config->base.state;
+
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i])
+			continue;
+
+		if (state->connector_states[i]->best_encoder == &encoder->base) {
+			found = to_intel_connector(state->connectors[i]);
 			break;
 		}
 	}
-- 
2.1.0

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

[PATCH 14/20] drm/i915: Don't use encoder->new_crtc in intel_lvds_compute_config()

[Ander Conselvan de Oliveira]

Move towards atomic by using the legacy modeset's drm_atomic_state
instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_lvds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 2b008b02..06d2da3 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -286,7 +286,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
 	struct intel_connector *intel_connector =
 		&lvds_encoder->attached_connector->base;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	unsigned int lvds_bpp;
 
 	/* Should never happen!! */
-- 
2.1.0

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

[PATCH 15/20] drm/i915: Pass an atomic state to modeset_global_resources() functions

[Ander Conselvan de Oliveira]

Follow up patches will convert some functions called from there to use
the atomic state, instead of directly accessing the new or current
config. This patch just changes the parameters, but shouldn't have any
functional changes.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 10 ++++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81f60b4..7c8b0ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -553,7 +553,7 @@ struct drm_i915_display_funcs {
 				 struct drm_crtc *crtc,
 				 uint32_t sprite_width, uint32_t sprite_height,
 				 int pixel_size, bool enable, bool scaled);
-	void (*modeset_global_resources)(struct drm_device *dev);
+	void (*modeset_global_resources)(struct drm_atomic_state *state);
 	/* Returns the active state of the crtc, and if the crtc is active,
 	 * fills out the pipe-config with the hw state. */
 	bool (*get_pipe_config)(struct intel_crtc *,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 895cf67..19e8928 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4825,8 +4825,9 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)
 	return mask;
 }
 
-static void modeset_update_crtc_power_domains(struct drm_device *dev)
+static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
 {
+	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
 	struct intel_crtc *crtc;
@@ -4848,7 +4849,7 @@ static void modeset_update_crtc_power_domains(struct drm_device *dev)
 	}
 
 	if (dev_priv->display.modeset_global_resources)
-		dev_priv->display.modeset_global_resources(dev);
+		dev_priv->display.modeset_global_resources(state);
 
 	for_each_intel_crtc(dev, crtc) {
 		enum intel_display_power_domain domain;
@@ -5096,8 +5097,9 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
 	WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
 }
 
-static void valleyview_modeset_global_resources(struct drm_device *dev)
+static void valleyview_modeset_global_resources(struct drm_atomic_state *state)
 {
+	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int max_pixclk = intel_mode_max_pixclk(dev_priv);
 	int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
@@ -11400,7 +11402,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	 * update the the output configuration. */
 	intel_modeset_update_state(dev, prepare_pipes);
 
-	modeset_update_crtc_power_domains(dev);
+	modeset_update_crtc_power_domains(pipe_config->base.state);
 
 	/* Set up the DPLL and any encoders state that needs to adjust or depend
 	 * on the DPLL.
-- 
2.1.0

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

[PATCH 16/20] drm/i915: Check lane sharing between pipes B & C using atomic state

[Ander Conselvan de Oliveira]

Makes that code atomic ready.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 19e8928..abfb7a9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5536,13 +5536,20 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
-static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
+static int pipe_required_fdi_lanes(struct drm_atomic_state *state,
+				   enum pipe pipe)
 {
 	struct intel_crtc *crtc =
-		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
+		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, pipe));
+	struct intel_crtc_state *crtc_state;
+
+	crtc_state = intel_atomic_get_crtc_state(state, crtc);
+	if (WARN_ON(IS_ERR(crtc_state))) {
+		/* Cause modeset to fail due to excess lanes. */
+		return 5;
+	}
 
-	if (crtc->base.state->enable &&
-	    crtc->config->has_pch_encoder)
+	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
 		return crtc->config->fdi_lanes;
 
 	return 0;
@@ -5551,6 +5558,8 @@ static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
 static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 				     struct intel_crtc_state *pipe_config)
 {
+	struct drm_atomic_state *state = pipe_config->base.state;
+
 	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
 		      pipe_name(pipe), pipe_config->fdi_lanes);
 	if (pipe_config->fdi_lanes > 4) {
@@ -5578,7 +5587,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 		return true;
 	case PIPE_B:
 		if (pipe_config->fdi_lanes > 2 &&
-		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
+		    pipe_required_fdi_lanes(state, PIPE_C) > 0) {
 			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
 				      pipe_name(pipe), pipe_config->fdi_lanes);
 			return false;
@@ -5590,7 +5599,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 				      pipe_name(pipe), pipe_config->fdi_lanes);
 			return false;
 		}
-		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
+		if (pipe_required_fdi_lanes(state, PIPE_B) > 2) {
 			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
 			return false;
 		}
@@ -5600,15 +5609,41 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 	}
 }
 
+static int add_pipe_b_c_to_state(struct drm_atomic_state *state)
+{
+	struct intel_crtc *pipe_B =
+		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_B));
+	struct intel_crtc *pipe_C =
+		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_C));
+	struct intel_crtc_state *crtc_state;
+
+	crtc_state = intel_atomic_get_crtc_state(state, pipe_B);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	crtc_state = intel_atomic_get_crtc_state(state, pipe_C);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	return 0;
+}
+
 #define RETRY 1
 static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 				       struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	int lane, link_bw, fdi_dotclock;
+	int lane, link_bw, fdi_dotclock, ret;
 	bool setup_ok, needs_recompute = false;
 
+	if (IS_IVYBRIDGE(dev) &&
+	    (intel_crtc->pipe == PIPE_B || intel_crtc->pipe == PIPE_C)) {
+		ret = add_pipe_b_c_to_state(pipe_config->base.state);
+		if (ret < 0)
+			return ret;
+	}
+
 retry:
 	/* FDI is a binary signal running at ~2.7GHz, encoding
 	 * each output octet as 10 bits. The actual frequency
-- 
2.1.0

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

[PATCH 17/20] drm/i915: Convert intel_pipe_will_have_type() to using atomic state

[Ander Conselvan de Oliveira]

Pass a crtc_state to it and find whether the pipe has an encoder of a
given type by looking at the drm_atomic_state the crtc_state points to.

Until recently i9xx_get_refclk() used to be called indirectly from
vlv_force_pll_on() with a dummy crtc_state. That dummy crtc state is not
converted to be part of a full drm atomic state, so add a WARN in case
someone decides to call that again with a such dummy state.

v2: Warn if there is no connectors for a given crtc. (Daniel)
    Replace comment i9xx_get_refclk() with a WARN_ON(). (Ander)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   2 +-
 drivers/gpu/drm/i915/intel_display.c | 133 +++++++++++++++++++++--------------
 2 files changed, 83 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c8b0ad..b71dec3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -544,7 +544,7 @@ struct drm_i915_display_funcs {
 	 * Returns true on success, false on failure.
 	 */
 	bool (*find_dpll)(const struct intel_limit *limit,
-			  struct intel_crtc *crtc,
+			  struct intel_crtc_state *crtc_state,
 			  int target, int refclk,
 			  struct dpll *match_clock,
 			  struct dpll *best_clock);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index abfb7a9..26f4d30 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -431,25 +431,41 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
  * intel_pipe_has_type() but looking at encoder->new_crtc instead of
  * encoder->crtc.
  */
-static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
+static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
+				      int type)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
+	int i, num_connectors = 0;
+
+	for (i = 0; i < state->num_connector; i++) {
+		if (!state->connectors[i])
+			continue;
+
+		connector_state = state->connector_states[i];
+		if (connector_state->crtc != crtc_state->base.crtc)
+			continue;
+
+		num_connectors++;
 
-	for_each_intel_encoder(dev, encoder)
-		if (encoder->new_crtc == crtc && encoder->type == type)
+		encoder = to_intel_encoder(connector_state->best_encoder);
+		if (encoder->type == type)
 			return true;
+	}
+
+	WARN_ON(num_connectors == 0);
 
 	return false;
 }
 
-static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
-						int refclk)
+static const intel_limit_t *
+intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev)) {
 			if (refclk == 100000)
 				limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -467,20 +483,21 @@ static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
 	return limit;
 }
 
-static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
+static const intel_limit_t *
+intel_g4x_limit(struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev))
 			limit = &intel_limits_g4x_dual_channel_lvds;
 		else
 			limit = &intel_limits_g4x_single_channel_lvds;
-	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
-		   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
+		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
 		limit = &intel_limits_g4x_hdmi;
-	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
 		limit = &intel_limits_g4x_sdvo;
 	} else /* The option is for other outputs */
 		limit = &intel_limits_i9xx_sdvo;
@@ -488,17 +505,18 @@ static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
 	return limit;
 }
 
-static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
+static const intel_limit_t *
+intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
 	if (HAS_PCH_SPLIT(dev))
-		limit = intel_ironlake_limit(crtc, refclk);
+		limit = intel_ironlake_limit(crtc_state, refclk);
 	else if (IS_G4X(dev)) {
-		limit = intel_g4x_limit(crtc);
+		limit = intel_g4x_limit(crtc_state);
 	} else if (IS_PINEVIEW(dev)) {
-		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_pineview_lvds;
 		else
 			limit = &intel_limits_pineview_sdvo;
@@ -507,14 +525,14 @@ static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
 	} else if (IS_VALLEYVIEW(dev)) {
 		limit = &intel_limits_vlv;
 	} else if (!IS_GEN2(dev)) {
-		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i9xx_lvds;
 		else
 			limit = &intel_limits_i9xx_sdvo;
 	} else {
-		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i8xx_lvds;
-		else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
+		else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
 			limit = &intel_limits_i8xx_dvo;
 		else
 			limit = &intel_limits_i8xx_dac;
@@ -601,15 +619,17 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
 }
 
 static bool
-i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+i9xx_find_best_dpll(const intel_limit_t *limit,
+		    struct intel_crtc_state *crtc_state,
 		    int target, int refclk, intel_clock_t *match_clock,
 		    intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	int err = target;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		/*
 		 * For LVDS just rely on its current settings for dual-channel.
 		 * We haven't figured out how to reliably set up different
@@ -662,15 +682,17 @@ i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+pnv_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	int err = target;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		/*
 		 * For LVDS just rely on its current settings for dual-channel.
 		 * We haven't figured out how to reliably set up different
@@ -721,10 +743,12 @@ pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+g4x_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	int max_n;
@@ -733,7 +757,7 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 	int err_most = (target >> 8) + (target >> 9);
 	found = false;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev))
 			clock.p2 = limit->p2.p2_fast;
 		else
@@ -778,10 +802,12 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+vlv_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	unsigned int bestppm = 1000000;
@@ -835,10 +861,12 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
 }
 
 static bool
-chv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
+chv_find_best_dpll(const intel_limit_t *limit,
+		   struct intel_crtc_state *crtc_state,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock;
 	uint64_t m2;
@@ -5725,7 +5753,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	 * - LVDS dual channel mode
 	 * - Double wide pipe
 	 */
-	if ((intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	if ((intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_LVDS) &&
 	     intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)
 		pipe_config->pipe_src_w &= ~1;
 
@@ -5904,15 +5932,18 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
-static int i9xx_get_refclk(struct intel_crtc *crtc, int num_connectors)
+static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
+			   int num_connectors)
 {
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int refclk;
 
+	WARN_ON(!crtc_state->base.state);
+
 	if (IS_VALLEYVIEW(dev)) {
 		refclk = 100000;
-	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
 		refclk = dev_priv->vbt.lvds_ssc_freq;
 		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
@@ -5955,7 +5986,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
 	crtc_state->dpll_hw_state.fp0 = fp;
 
 	crtc->lowfreq_avail = false;
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    reduced_clock && i915.powersave) {
 		crtc_state->dpll_hw_state.fp1 = fp2;
 		crtc->lowfreq_avail = true;
@@ -6313,6 +6344,7 @@ void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
 	struct intel_crtc *crtc =
 		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
 	struct intel_crtc_state pipe_config = {
+		.base.crtc = &crtc->base,
 		.pixel_multiplier = 1,
 		.dpll = *dpll,
 	};
@@ -6357,12 +6389,12 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
 
 	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
 
-	is_sdvo = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO) ||
-		intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI);
+	is_sdvo = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO) ||
+		intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI);
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 		dpll |= DPLLB_MODE_LVDS;
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
@@ -6405,7 +6437,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
 
 	if (crtc_state->sdvo_tv_clock)
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
-	else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -6435,7 +6467,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
 	} else {
 		if (clock->p1 == 2)
@@ -6446,10 +6478,10 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
 			dpll |= PLL_P2_DIVIDE_BY_4;
 	}
 
-	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
+	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
 		dpll |= DPLL_DVO_2X_MODE;
 
-	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -6686,7 +6718,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 		return 0;
 
 	if (!crtc_state->clock_set) {
-		refclk = i9xx_get_refclk(crtc, num_connectors);
+		refclk = i9xx_get_refclk(crtc_state, num_connectors);
 
 		/*
 		 * Returns a set of divisors for the desired target clock with
@@ -6694,8 +6726,8 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
 		 * 2) / p1 / p2.
 		 */
-		limit = intel_limit(crtc, refclk);
-		ok = dev_priv->display.find_dpll(limit, crtc,
+		limit = intel_limit(crtc_state, refclk);
+		ok = dev_priv->display.find_dpll(limit, crtc_state,
 						 crtc_state->port_clock,
 						 refclk, NULL, &clock);
 		if (!ok) {
@@ -6711,7 +6743,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 			 * we will disable the LVDS downclock feature.
 			 */
 			has_reduced_clock =
-				dev_priv->display.find_dpll(limit, crtc,
+				dev_priv->display.find_dpll(limit, crtc_state,
 							    dev_priv->lvds_downclock,
 							    refclk, &clock,
 							    &reduced_clock);
@@ -7539,12 +7571,11 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int refclk;
 	const intel_limit_t *limit;
 	bool ret, is_lvds = false;
 
-	is_lvds = intel_pipe_will_have_type(intel_crtc, INTEL_OUTPUT_LVDS);
+	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
 
 	refclk = ironlake_get_refclk(crtc);
 
@@ -7553,8 +7584,8 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	 * refclk, or FALSE.  The returned values represent the clock equation:
 	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
 	 */
-	limit = intel_limit(intel_crtc, refclk);
-	ret = dev_priv->display.find_dpll(limit, intel_crtc,
+	limit = intel_limit(crtc_state, refclk);
+	ret = dev_priv->display.find_dpll(limit, crtc_state,
 					  crtc_state->port_clock,
 					  refclk, NULL, clock);
 	if (!ret)
@@ -7568,7 +7599,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 		 * downclock feature.
 		*/
 		*has_reduced_clock =
-			dev_priv->display.find_dpll(limit, intel_crtc,
+			dev_priv->display.find_dpll(limit, crtc_state,
 						    dev_priv->lvds_downclock,
 						    refclk, clock,
 						    reduced_clock);
-- 
2.1.0

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

[PATCH 18/20] drm/i915: Don't look at staged config crtc when changing DRRS state

[Ander Conselvan de Oliveira]

The function intel_dp_set_drrs_state() would decide which pipe to
downclock based on the staged config for the given connector. However,
the result of that function is immediate, and it uses input values from
crtc->config, so it should be looking at the current crtc instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0c61aed..22bf6f2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4935,7 +4935,7 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 
 	dig_port = dp_to_dig_port(intel_dp);
 	encoder = &dig_port->base;
-	intel_crtc = encoder->new_crtc;
+	intel_crtc = to_intel_crtc(encoder->base.crtc);
 
 	if (!intel_crtc) {
 		DRM_DEBUG_KMS("DRRS: intel_crtc not initialized\n");
-- 
2.1.0

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

[PATCH 19/20] drm/i915: Remove usage of encoder->new_crtc from clock computations

[Ander Conselvan de Oliveira]

Some of the crtc_compute_clock() still depended on encoder->new_crtc
since they didn't use intel_pipe_will_have_type() and used an open
coded version of that function instead. This patch replaces those with
the appropriate code that checks the atomic state intead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 26f4d30..89299b6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6695,11 +6695,18 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	bool is_lvds = false, is_dsi = false;
 	struct intel_encoder *encoder;
 	const intel_limit_t *limit;
+	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector_state *connector_state;
+	int i;
 
-	for_each_intel_encoder(dev, encoder) {
-		if (encoder->new_crtc != crtc)
+	for (i = 0; i < state->num_connector; i++) {
+		connector_state = state->connector_states[i];
+		if (!state->connectors[i] ||
+		    connector_state->crtc != &crtc->base)
 			continue;
 
+		encoder = to_intel_encoder(connector_state->best_encoder);
+
 		switch (encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
@@ -7373,18 +7380,24 @@ void intel_init_pch_refclk(struct drm_device *dev)
 		lpt_init_pch_refclk(dev);
 }
 
-static int ironlake_get_refclk(struct drm_crtc *crtc)
+static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
-	int num_connectors = 0;
+	int num_connectors = 0, i;
 	bool is_lvds = false;
 
-	for_each_intel_encoder(dev, encoder) {
-		if (encoder->new_crtc != to_intel_crtc(crtc))
+	for (i = 0; i < state->num_connector; i++) {
+		connector_state = state->connector_states[i];
+		if (!state->connectors[i] ||
+		    connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
+		encoder = to_intel_encoder(connector_state->best_encoder);
+
 		switch (encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
@@ -7577,7 +7590,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 
 	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
 
-	refclk = ironlake_get_refclk(crtc);
+	refclk = ironlake_get_refclk(crtc_state);
 
 	/*
 	 * Returns a set of divisors for the desired target clock with the given
@@ -7632,16 +7645,22 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	struct drm_crtc *crtc = &intel_crtc->base;
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_encoder *intel_encoder;
+	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector_state *connector_state;
+	struct intel_encoder *encoder;
 	uint32_t dpll;
-	int factor, num_connectors = 0;
+	int factor, num_connectors = 0, i;
 	bool is_lvds = false, is_sdvo = false;
 
-	for_each_intel_encoder(dev, intel_encoder) {
-		if (intel_encoder->new_crtc != to_intel_crtc(crtc))
+	for (i = 0; i < state->num_connector; i++) {
+		connector_state = state->connector_states[i];
+		if (!state->connectors[i] ||
+		    connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
-		switch (intel_encoder->type) {
+		encoder = to_intel_encoder(connector_state->best_encoder);
+
+		switch (encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
 			break;
-- 
2.1.0

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

Re: [PATCH 05/20] drm/i915: Update dummy connector atomic state with current config

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:05PM +0200, Ander Conselvan de Oliveira wrote:
> @@ -10136,6 +10157,8 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
>  		crtc->base.state->enable = crtc->new_enabled;
>  		crtc->base.enabled = crtc->new_enabled;
>  	}
> +
> +	intel_modeset_update_connector_atomic_state(dev);

Just an aside: I think we should move the crtc state update and the
updated vblank timings right before the call to intel_modeset_update_state
into that itself. I spent a bit of time looking for that logic until I
found it ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/20] drm/i915: Don't use encoder->new_crtc in intel_modeset_pipe_config()

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:08PM +0200, Ander Conselvan de Oliveira wrote:
> Move towards atomic by using the legacy modeset's drm_atomic_state
> instead.
> 
> v2: Move call to drm_atomic_add_affected_connectors() to
>     intel_modeset_compute_config(). (Daniel)
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b76726f..7851d6c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10434,8 +10434,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct intel_encoder *encoder;
> +	struct intel_connector *connector;
> +	struct drm_connector_state *connector_state;
>  	struct intel_crtc_state *pipe_config;
>  	int plane_bpp, ret = -EINVAL;
> +	int i;
>  	bool retry = true;
>  
>  	if (!check_encoder_cloning(to_intel_crtc(crtc))) {
> @@ -10509,11 +10512,17 @@ encoder_retry:
>  	 * adjust it according to limitations or connector properties, and also
>  	 * a chance to reject the mode entirely.
>  	 */
> -	for_each_intel_encoder(dev, encoder) {
> +	for (i = 0; i < state->num_connector; i++) {
> +		connector = to_intel_connector(state->connectors[i]);
> +		if (!connector)
> +			continue;
>  
> -		if (&encoder->new_crtc->base != crtc)
> +		connector_state = state->connector_states[i];
> +		if (connector_state->crtc != crtc)
>  			continue;
>  
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
>  		if (!(encoder->compute_config(encoder, pipe_config))) {
>  			DRM_DEBUG_KMS("Encoder config failure\n");
>  			goto fail;
> @@ -11237,6 +11246,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
>  	struct intel_crtc *intel_crtc;
>  	int ret = 0;
>  
> +	ret = drm_atomic_add_affected_connectors(state, crtc);

Ah, here's the usage of ret that I've missed in the first patch.
-Daniel

> +	if (ret)
> +		return ERR_PTR(ret);
> +
>  	intel_modeset_affected_pipes(crtc, modeset_pipes,
>  				     prepare_pipes, disable_pipes);
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/20] drm/i915: Don't use encoder->new_crtc in compute_baseline_pipe_bpp()

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:09PM +0200, Ander Conselvan de Oliveira wrote:
> Move towards atomic by using the legacy modeset's drm_atomic_state
> instead.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7851d6c..895cf67 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10215,8 +10215,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
>  			  struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_device *dev = crtc->base.dev;
> +	struct drm_atomic_state *state;
>  	struct intel_connector *connector;
> -	int bpp;
> +	int bpp, i;
>  
>  	switch (fb->pixel_format) {
>  	case DRM_FORMAT_C8:
> @@ -10256,10 +10257,13 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
>  
>  	pipe_config->pipe_bpp = bpp;
>  
> +	state = pipe_config->base.state;
> +
>  	/* Clamp display bpp to EDID value */
> -	for_each_intel_connector(dev, connector) {
> -		if (!connector->new_encoder ||
> -		    connector->new_encoder->new_crtc != crtc)
> +	for (i = 0; i < state->num_connector; i++) {
> +		connector = to_intel_connector(state->connectors[i]);
> +		if (!connector ||

Bikeshed: I think we should have the !connector case as a separate one,
since someone we should add a for_each_connector_in_state loop which
integrates the continue. Frobbed while applying.
-Daniel

> +		    state->connector_states[i]->crtc != &crtc->base)
>  			continue;
>  
>  		connected_sink_compute_bpp(connector, pipe_config);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/20] drm/i915: Don't use encoder->new_crtc in compute_baseline_pipe_bpp()

[Daniel Vetter]

On Thu, Mar 26, 2015 at 05:46:22PM +0100, Daniel Vetter wrote:
> On Fri, Mar 20, 2015 at 04:18:09PM +0200, Ander Conselvan de Oliveira wrote:
> > Move towards atomic by using the legacy modeset's drm_atomic_state
> > instead.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 7851d6c..895cf67 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10215,8 +10215,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
> >  			  struct intel_crtc_state *pipe_config)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_atomic_state *state;
> >  	struct intel_connector *connector;
> > -	int bpp;
> > +	int bpp, i;
> >  
> >  	switch (fb->pixel_format) {
> >  	case DRM_FORMAT_C8:
> > @@ -10256,10 +10257,13 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
> >  
> >  	pipe_config->pipe_bpp = bpp;
> >  
> > +	state = pipe_config->base.state;
> > +
> >  	/* Clamp display bpp to EDID value */
> > -	for_each_intel_connector(dev, connector) {
> > -		if (!connector->new_encoder ||
> > -		    connector->new_encoder->new_crtc != crtc)
> > +	for (i = 0; i < state->num_connector; i++) {
> > +		connector = to_intel_connector(state->connectors[i]);
> > +		if (!connector ||
> 
> Bikeshed: I think we should have the !connector case as a separate one,
> since someone we should add a for_each_connector_in_state loop which
> integrates the continue. Frobbed while applying.

Even better justification: Checking state->connectors[i] is safer because
of the upcast.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/20] drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder()

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:12PM +0200, Ander Conselvan de Oliveira wrote:
> Instead of using connector->new_encoder, get the same information from
> the pipe_config, thus making the function ready for the atomic
> conversion.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 8aee7d7..47b9307 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -492,17 +492,23 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
>  }
>  
>  static struct intel_encoder *
> -intel_ddi_get_crtc_new_encoder(struct intel_crtc *crtc)
> +intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct intel_encoder *intel_encoder, *ret = NULL;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct intel_encoder *ret = NULL;
> +	struct drm_atomic_state *state;
>  	int num_encoders = 0;
> +	int i;
>  
> -	for_each_intel_encoder(dev, intel_encoder) {
> -		if (intel_encoder->new_crtc == crtc) {
> -			ret = intel_encoder;
> -			num_encoders++;
> -		}
> +	state = crtc_state->base.state;
> +
> +	for (i = 0; i < state->num_connector; i++) {
> +		if (!state->connectors[i] ||
> +		    state->connector_states[i]->crtc != crtc_state->base.crtc)
> +			continue;
> +
> +		ret = to_intel_encoder(state->connector_states[i]->best_encoder);
> +		num_encoders++;
>  	}
>  
>  	WARN(num_encoders != 1, "%d encoders on crtc for pipe %c\n", num_encoders,
> @@ -1216,7 +1222,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct intel_encoder *intel_encoder =
> -		intel_ddi_get_crtc_new_encoder(intel_crtc);
> +		intel_ddi_get_crtc_new_encoder(crtc_state);

I think for atomic we need to change this further - the only reason we
want to look at the encoder is to figure out the type. And that can
actually change at runtime (hooray). What we really need is a
ddi_personality enum in the pipe_config which is set once in the encoder
compute_config stuff and then used everywhere in the ddi modeset code
where we currently use at encoder->type.

But that's an entire different project, this patch here is a suitable
direct conversion. Not sure whether we have a jira for this part already,
certainly should update the one that talks about modesets vs. probe races
(which this kinda is). There's also been multiple different patchsets
floating around that all got bikeshedded a bit somehow.
-Daniel

>  	int clock = crtc_state->port_clock;
>  
>  	if (IS_SKYLAKE(dev))
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 13/20] drm/i915: Don't use staged config in intel_dp_mst_compute_config()

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:13PM +0200, Ander Conselvan de Oliveira wrote:
> Move towards atomic by using the legacy modeset's drm_atomic_state
> instead.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 5c06a06..b132fe6 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -36,11 +36,11 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
>  	struct intel_digital_port *intel_dig_port = intel_mst->primary;
>  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> -	struct drm_device *dev = encoder->base.dev;
> -	int bpp;
> +	struct drm_atomic_state *state;
> +	int bpp, i;
>  	int lane_count, slots;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> -	struct intel_connector *found = NULL, *intel_connector;
> +	struct intel_connector *found = NULL;
>  	int mst_pbn;
>  
>  	pipe_config->dp_encoder_is_mst = true;
> @@ -58,9 +58,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
>  	pipe_config->pipe_bpp = 24;
>  	pipe_config->port_clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
>  
> -	for_each_intel_connector(dev, intel_connector) {
> -		if (intel_connector->new_encoder == encoder) {
> -			found = intel_connector;
> +	state = pipe_config->base.state;
> +
> +	for (i = 0; i < state->num_connector; i++) {
> +		if (!state->connectors[i])
> +			continue;
> +
> +		if (state->connector_states[i]->best_encoder == &encoder->base) {
> +			found = to_intel_connector(state->connectors[i]);

A helper to look up the connector/connector_state for a given encoder
might be useful in general. We also need that for the ddi personality fun,
and I guess a bunch of other places where we have loops just because our
logic is based on encoders but atomic states are attached to encoders.

Not sure if it's worth it thought.
-Daniel

>  			break;
>  		}
>  	}
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/20] drm/i915: Allocate a drm_atomic_state for the legacy modeset code

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:03PM +0200, Ander Conselvan de Oliveira wrote:
> @@ -8924,6 +8932,11 @@ retry:
>  	else
>  		intel_crtc->new_config = NULL;
>  fail_unlock:
> +	if (state) {
> +		drm_atomic_state_free(state);
> +		state = NULL;
> +	}

I think we should push the NULL handling into drm_atomic_state_free like
with kfree. Can you please do such a patch on top of this for all of drm?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/20] drm/i915: Allocate a drm_atomic_state for the legacy modeset code

[Ander Conselvan De Oliveira]

On Fri, 2015-03-27 at 10:01 +0100, Daniel Vetter wrote:
> On Fri, Mar 20, 2015 at 04:18:03PM +0200, Ander Conselvan de Oliveira wrote:
> > @@ -8924,6 +8932,11 @@ retry:
> >  	else
> >  		intel_crtc->new_config = NULL;
> >  fail_unlock:
> > +	if (state) {
> > +		drm_atomic_state_free(state);
> > +		state = NULL;
> > +	}
> 
> I think we should push the NULL handling into drm_atomic_state_free like
> with kfree. Can you please do such a patch on top of this for all of drm?

Sure, I'll do that.

Ander

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

Re: [PATCH 16/20] drm/i915: Check lane sharing between pipes B & C using atomic state

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:16PM +0200, Ander Conselvan de Oliveira wrote:
> Makes that code atomic ready.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++++++++++++++++++++------
>  1 file changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 19e8928..abfb7a9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5536,13 +5536,20 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
>  	return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> -static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
> +static int pipe_required_fdi_lanes(struct drm_atomic_state *state,
> +				   enum pipe pipe)
>  {
>  	struct intel_crtc *crtc =
> -		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> +		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, pipe));
> +	struct intel_crtc_state *crtc_state;
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, crtc);
> +	if (WARN_ON(IS_ERR(crtc_state))) {
> +		/* Cause modeset to fail due to excess lanes. */
> +		return 5;
> +	}

Why that? If you can't get at the state it simply might be due to ww-mutex
deadlock. The correct way to handle that is to pass the error all down the
callchain. Atm this isn't a problem since we always grab all locks, but
without this error passing the code isn't strictly atomic ready yet.


>  
> -	if (crtc->base.state->enable &&
> -	    crtc->config->has_pch_encoder)
> +	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
>  		return crtc->config->fdi_lanes;
>  
>  	return 0;
> @@ -5551,6 +5558,8 @@ static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
>  static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  				     struct intel_crtc_state *pipe_config)
>  {
> +	struct drm_atomic_state *state = pipe_config->base.state;
> +
>  	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
>  		      pipe_name(pipe), pipe_config->fdi_lanes);
>  	if (pipe_config->fdi_lanes > 4) {
> @@ -5578,7 +5587,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  		return true;
>  	case PIPE_B:
>  		if (pipe_config->fdi_lanes > 2 &&
> -		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
> +		    pipe_required_fdi_lanes(state, PIPE_C) > 0) {
>  			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
>  			return false;
> @@ -5590,7 +5599,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
>  			return false;
>  		}
> -		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
> +		if (pipe_required_fdi_lanes(state, PIPE_B) > 2) {
>  			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
>  			return false;
>  		}
> @@ -5600,15 +5609,41 @@ static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  	}
>  }
>  
> +static int add_pipe_b_c_to_state(struct drm_atomic_state *state)
> +{
> +	struct intel_crtc *pipe_B =
> +		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_B));
> +	struct intel_crtc *pipe_C =
> +		to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_C));
> +	struct intel_crtc_state *crtc_state;
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, pipe_B);
> +	if (IS_ERR(crtc_state))
> +		return PTR_ERR(crtc_state);
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, pipe_C);
> +	if (IS_ERR(crtc_state))
> +		return PTR_ERR(crtc_state);
> +
> +	return 0;
> +}

Ah maybe I should read on with the code first ;-)

Imo for atomic check functions it's better to grab additional state
objects late and wire the error code throughs. Two benefits for that
approach over yours here where you grab needed states upfront:
- You only acquire locks when really needed - e.g. on ivb we only need the
  other states for pipes B&C.
- There's no split between where you acquire a state object and where you
  really need it, hence no need for WARN_ON and friends to make the code
  robust.

The downside is ofc that you need to thread the error code through
callchains. But you also need to thread the -EINVAL back, so ime from
writing atomic helpers it's in practice not more work.

I'll punt on this patch for now until we've discussed this a bit.
-Daniel

> +
>  #define RETRY 1
>  static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  				       struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> -	int lane, link_bw, fdi_dotclock;
> +	int lane, link_bw, fdi_dotclock, ret;
>  	bool setup_ok, needs_recompute = false;
>  
> +	if (IS_IVYBRIDGE(dev) &&
> +	    (intel_crtc->pipe == PIPE_B || intel_crtc->pipe == PIPE_C)) {
> +		ret = add_pipe_b_c_to_state(pipe_config->base.state);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  retry:
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
>  	 * each output octet as 10 bits. The actual frequency
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 17/20] drm/i915: Convert intel_pipe_will_have_type() to using atomic state

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:17PM +0200, Ander Conselvan de Oliveira wrote:
> Pass a crtc_state to it and find whether the pipe has an encoder of a
> given type by looking at the drm_atomic_state the crtc_state points to.
> 
> Until recently i9xx_get_refclk() used to be called indirectly from
> vlv_force_pll_on() with a dummy crtc_state. That dummy crtc state is not
> converted to be part of a full drm atomic state, so add a WARN in case
> someone decides to call that again with a such dummy state.
> 
> v2: Warn if there is no connectors for a given crtc. (Daniel)
>     Replace comment i9xx_get_refclk() with a WARN_ON(). (Ander)

Interesting story. I've dug out the relevant commits and added a note
while merging.
-Daniel

> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |   2 +-
>  drivers/gpu/drm/i915/intel_display.c | 133 +++++++++++++++++++++--------------
>  2 files changed, 83 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7c8b0ad..b71dec3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -544,7 +544,7 @@ struct drm_i915_display_funcs {
>  	 * Returns true on success, false on failure.
>  	 */
>  	bool (*find_dpll)(const struct intel_limit *limit,
> -			  struct intel_crtc *crtc,
> +			  struct intel_crtc_state *crtc_state,
>  			  int target, int refclk,
>  			  struct dpll *match_clock,
>  			  struct dpll *best_clock);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index abfb7a9..26f4d30 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -431,25 +431,41 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
>   * intel_pipe_has_type() but looking at encoder->new_crtc instead of
>   * encoder->crtc.
>   */
> -static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
> +static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
> +				      int type)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_atomic_state *state = crtc_state->base.state;
> +	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
> +	int i, num_connectors = 0;
> +
> +	for (i = 0; i < state->num_connector; i++) {
> +		if (!state->connectors[i])
> +			continue;
> +
> +		connector_state = state->connector_states[i];
> +		if (connector_state->crtc != crtc_state->base.crtc)
> +			continue;
> +
> +		num_connectors++;
>  
> -	for_each_intel_encoder(dev, encoder)
> -		if (encoder->new_crtc == crtc && encoder->type == type)
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +		if (encoder->type == type)
>  			return true;
> +	}
> +
> +	WARN_ON(num_connectors == 0);
>  
>  	return false;
>  }
>  
> -static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
> -						int refclk)
> +static const intel_limit_t *
> +intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		if (intel_is_dual_link_lvds(dev)) {
>  			if (refclk == 100000)
>  				limit = &intel_limits_ironlake_dual_lvds_100m;
> @@ -467,20 +483,21 @@ static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
>  	return limit;
>  }
>  
> -static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
> +static const intel_limit_t *
> +intel_g4x_limit(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		if (intel_is_dual_link_lvds(dev))
>  			limit = &intel_limits_g4x_dual_channel_lvds;
>  		else
>  			limit = &intel_limits_g4x_single_channel_lvds;
> -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
> -		   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
> +		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
>  		limit = &intel_limits_g4x_hdmi;
> -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
>  		limit = &intel_limits_g4x_sdvo;
>  	} else /* The option is for other outputs */
>  		limit = &intel_limits_i9xx_sdvo;
> @@ -488,17 +505,18 @@ static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
>  	return limit;
>  }
>  
> -static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
> +static const intel_limit_t *
> +intel_limit(struct intel_crtc_state *crtc_state, int refclk)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
>  
>  	if (HAS_PCH_SPLIT(dev))
> -		limit = intel_ironlake_limit(crtc, refclk);
> +		limit = intel_ironlake_limit(crtc_state, refclk);
>  	else if (IS_G4X(dev)) {
> -		limit = intel_g4x_limit(crtc);
> +		limit = intel_g4x_limit(crtc_state);
>  	} else if (IS_PINEVIEW(dev)) {
> -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_pineview_lvds;
>  		else
>  			limit = &intel_limits_pineview_sdvo;
> @@ -507,14 +525,14 @@ static const intel_limit_t *intel_limit(struct intel_crtc *crtc, int refclk)
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		limit = &intel_limits_vlv;
>  	} else if (!IS_GEN2(dev)) {
> -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_i9xx_lvds;
>  		else
>  			limit = &intel_limits_i9xx_sdvo;
>  	} else {
> -		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_i8xx_lvds;
> -		else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
> +		else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
>  			limit = &intel_limits_i8xx_dvo;
>  		else
>  			limit = &intel_limits_i8xx_dac;
> @@ -601,15 +619,17 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
>  }
>  
>  static bool
> -i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +i9xx_find_best_dpll(const intel_limit_t *limit,
> +		    struct intel_crtc_state *crtc_state,
>  		    int target, int refclk, intel_clock_t *match_clock,
>  		    intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	int err = target;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		/*
>  		 * For LVDS just rely on its current settings for dual-channel.
>  		 * We haven't figured out how to reliably set up different
> @@ -662,15 +682,17 @@ i9xx_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
>  }
>  
>  static bool
> -pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +pnv_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	int err = target;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		/*
>  		 * For LVDS just rely on its current settings for dual-channel.
>  		 * We haven't figured out how to reliably set up different
> @@ -721,10 +743,12 @@ pnv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
>  }
>  
>  static bool
> -g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +g4x_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	int max_n;
> @@ -733,7 +757,7 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
>  	int err_most = (target >> 8) + (target >> 9);
>  	found = false;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		if (intel_is_dual_link_lvds(dev))
>  			clock.p2 = limit->p2.p2_fast;
>  		else
> @@ -778,10 +802,12 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
>  }
>  
>  static bool
> -vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +vlv_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	unsigned int bestppm = 1000000;
> @@ -835,10 +861,12 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
>  }
>  
>  static bool
> -chv_find_best_dpll(const intel_limit_t *limit, struct intel_crtc *crtc,
> +chv_find_best_dpll(const intel_limit_t *limit,
> +		   struct intel_crtc_state *crtc_state,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	intel_clock_t clock;
>  	uint64_t m2;
> @@ -5725,7 +5753,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  	 * - LVDS dual channel mode
>  	 * - Double wide pipe
>  	 */
> -	if ((intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if ((intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_LVDS) &&
>  	     intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)
>  		pipe_config->pipe_src_w &= ~1;
>  
> @@ -5904,15 +5932,18 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
>  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
>  }
>  
> -static int i9xx_get_refclk(struct intel_crtc *crtc, int num_connectors)
> +static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
> +			   int num_connectors)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int refclk;
>  
> +	WARN_ON(!crtc_state->base.state);
> +
>  	if (IS_VALLEYVIEW(dev)) {
>  		refclk = 100000;
> -	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
>  		refclk = dev_priv->vbt.lvds_ssc_freq;
>  		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> @@ -5955,7 +5986,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
>  	crtc_state->dpll_hw_state.fp0 = fp;
>  
>  	crtc->lowfreq_avail = false;
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  	    reduced_clock && i915.powersave) {
>  		crtc_state->dpll_hw_state.fp1 = fp2;
>  		crtc->lowfreq_avail = true;
> @@ -6313,6 +6344,7 @@ void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe,
>  	struct intel_crtc *crtc =
>  		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
>  	struct intel_crtc_state pipe_config = {
> +		.base.crtc = &crtc->base,
>  		.pixel_multiplier = 1,
>  		.dpll = *dpll,
>  	};
> @@ -6357,12 +6389,12 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>  
>  	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
>  
> -	is_sdvo = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO) ||
> -		intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI);
> +	is_sdvo = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO) ||
> +		intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI);
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  		dpll |= DPLLB_MODE_LVDS;
>  	else
>  		dpll |= DPLLB_MODE_DAC_SERIAL;
> @@ -6405,7 +6437,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>  
>  	if (crtc_state->sdvo_tv_clock)
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
> -	else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -6435,7 +6467,7 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
>  		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
>  	} else {
>  		if (clock->p1 == 2)
> @@ -6446,10 +6478,10 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>  			dpll |= PLL_P2_DIVIDE_BY_4;
>  	}
>  
> -	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
> +	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
>  		dpll |= DPLL_DVO_2X_MODE;
>  
> -	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -6686,7 +6718,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  		return 0;
>  
>  	if (!crtc_state->clock_set) {
> -		refclk = i9xx_get_refclk(crtc, num_connectors);
> +		refclk = i9xx_get_refclk(crtc_state, num_connectors);
>  
>  		/*
>  		 * Returns a set of divisors for the desired target clock with
> @@ -6694,8 +6726,8 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
>  		 * 2) / p1 / p2.
>  		 */
> -		limit = intel_limit(crtc, refclk);
> -		ok = dev_priv->display.find_dpll(limit, crtc,
> +		limit = intel_limit(crtc_state, refclk);
> +		ok = dev_priv->display.find_dpll(limit, crtc_state,
>  						 crtc_state->port_clock,
>  						 refclk, NULL, &clock);
>  		if (!ok) {
> @@ -6711,7 +6743,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  			 * we will disable the LVDS downclock feature.
>  			 */
>  			has_reduced_clock =
> -				dev_priv->display.find_dpll(limit, crtc,
> +				dev_priv->display.find_dpll(limit, crtc_state,
>  							    dev_priv->lvds_downclock,
>  							    refclk, &clock,
>  							    &reduced_clock);
> @@ -7539,12 +7571,11 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int refclk;
>  	const intel_limit_t *limit;
>  	bool ret, is_lvds = false;
>  
> -	is_lvds = intel_pipe_will_have_type(intel_crtc, INTEL_OUTPUT_LVDS);
> +	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
>  
>  	refclk = ironlake_get_refclk(crtc);
>  
> @@ -7553,8 +7584,8 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  	 * refclk, or FALSE.  The returned values represent the clock equation:
>  	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
>  	 */
> -	limit = intel_limit(intel_crtc, refclk);
> -	ret = dev_priv->display.find_dpll(limit, intel_crtc,
> +	limit = intel_limit(crtc_state, refclk);
> +	ret = dev_priv->display.find_dpll(limit, crtc_state,
>  					  crtc_state->port_clock,
>  					  refclk, NULL, clock);
>  	if (!ret)
> @@ -7568,7 +7599,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  		 * downclock feature.
>  		*/
>  		*has_reduced_clock =
> -			dev_priv->display.find_dpll(limit, intel_crtc,
> +			dev_priv->display.find_dpll(limit, crtc_state,
>  						    dev_priv->lvds_downclock,
>  						    refclk, clock,
>  						    reduced_clock);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 18/20] drm/i915: Don't look at staged config crtc when changing DRRS state

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:18PM +0200, Ander Conselvan de Oliveira wrote:
> The function intel_dp_set_drrs_state() would decide which pipe to
> downclock based on the staged config for the given connector. However,
> the result of that function is immediate, and it uses input values from
> crtc->config, so it should be looking at the current crtc instead.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0c61aed..22bf6f2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4935,7 +4935,7 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>  
>  	dig_port = dp_to_dig_port(intel_dp);
>  	encoder = &dig_port->base;
> -	intel_crtc = encoder->new_crtc;
> +	intel_crtc = to_intel_crtc(encoder->base.crtc);

drm_encoder->crtc is kinda deprecated too. Just not our own kind of
deprecated, since there's still some random users in the core. But I do
plan to eventually leave them all as NULL for atomic drivers, to make sure
everyone is exclusively looking at the state objects.

Anyway that's for later.
-Daniel

>  
>  	if (!intel_crtc) {
>  		DRM_DEBUG_KMS("DRRS: intel_crtc not initialized\n");
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 19/20] drm/i915: Remove usage of encoder->new_crtc from clock computations

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:19PM +0200, Ander Conselvan de Oliveira wrote:
> Some of the crtc_compute_clock() still depended on encoder->new_crtc
> since they didn't use intel_pipe_will_have_type() and used an open
> coded version of that function instead. This patch replaces those with
> the appropriate code that checks the atomic state intead.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++++++++++++-----------
>  1 file changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 26f4d30..89299b6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6695,11 +6695,18 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	bool is_lvds = false, is_dsi = false;
>  	struct intel_encoder *encoder;
>  	const intel_limit_t *limit;
> +	struct drm_atomic_state *state = crtc_state->base.state;
> +	struct drm_connector_state *connector_state;
> +	int i;
>  
> -	for_each_intel_encoder(dev, encoder) {
> -		if (encoder->new_crtc != crtc)
> +	for (i = 0; i < state->num_connector; i++) {
> +		connector_state = state->connector_states[i];
> +		if (!state->connectors[i] ||

Another case for a for_each_connector_in_state macro. I'll do the same
bikeshed as with the others.
-Daniel


> +		    connector_state->crtc != &crtc->base)
>  			continue;
>  
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
>  		switch (encoder->type) {
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
> @@ -7373,18 +7380,24 @@ void intel_init_pch_refclk(struct drm_device *dev)
>  		lpt_init_pch_refclk(dev);
>  }
>  
> -static int ironlake_get_refclk(struct drm_crtc *crtc)
> +static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_atomic_state *state = crtc_state->base.state;
> +	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
> -	int num_connectors = 0;
> +	int num_connectors = 0, i;
>  	bool is_lvds = false;
>  
> -	for_each_intel_encoder(dev, encoder) {
> -		if (encoder->new_crtc != to_intel_crtc(crtc))
> +	for (i = 0; i < state->num_connector; i++) {
> +		connector_state = state->connector_states[i];
> +		if (!state->connectors[i] ||
> +		    connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
>  		switch (encoder->type) {
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
> @@ -7577,7 +7590,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  
>  	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
>  
> -	refclk = ironlake_get_refclk(crtc);
> +	refclk = ironlake_get_refclk(crtc_state);
>  
>  	/*
>  	 * Returns a set of divisors for the desired target clock with the given
> @@ -7632,16 +7645,22 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	struct drm_crtc *crtc = &intel_crtc->base;
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_encoder *intel_encoder;
> +	struct drm_atomic_state *state = crtc_state->base.state;
> +	struct drm_connector_state *connector_state;
> +	struct intel_encoder *encoder;
>  	uint32_t dpll;
> -	int factor, num_connectors = 0;
> +	int factor, num_connectors = 0, i;
>  	bool is_lvds = false, is_sdvo = false;
>  
> -	for_each_intel_encoder(dev, intel_encoder) {
> -		if (intel_encoder->new_crtc != to_intel_crtc(crtc))
> +	for (i = 0; i < state->num_connector; i++) {
> +		connector_state = state->connector_states[i];
> +		if (!state->connectors[i] ||
> +		    connector_state->crtc != crtc_state->base.crtc)
>  			continue;
>  
> -		switch (intel_encoder->type) {
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
> +		switch (encoder->type) {
>  		case INTEL_OUTPUT_LVDS:
>  			is_lvds = true;
>  			break;
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 00/20] Remove depencies on staged config for atomic transition

[Daniel Vetter]

On Fri, Mar 20, 2015 at 04:18:00PM +0200, Ander Conselvan de Oliveira wrote:
> Version 3 of the series with comments from Chandra addressed. I'm sending the
> whole series again so it goes through another round of PRTS testing.
> 
> Thanks,
> Ander
> 
> Ander Conselvan de Oliveira (19):
>   drm/i915: Add intel_atomic_get_crtc_state() helper function
>   drm/i915: Pass acquire ctx also to intel_release_load_detect_pipe()
>   drm/i915: Allocate a drm_atomic_state for the legacy modeset code
>   drm/i915: Allocate a crtc_state also when the crtc is being disabled
>   drm/i915: Update dummy connector atomic state with current config
>   drm/i915: Implement connector state duplication
>   drm/i915: Copy the staged connector config to the legacy atomic state
>   drm/i915: Don't use encoder->new_crtc in intel_modeset_pipe_config()
>   drm/i915: Don't use encoder->new_crtc in compute_baseline_pipe_bpp()
>   drm/i915: Don't depend on encoder->new_crtc in
>     intel_dp_compute_config()
>   drm/i915: Don't depend on encoder->new_crtc in
>     intel_hdmi_compute_config
>   drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder()
>   drm/i915: Don't use staged config in intel_dp_mst_compute_config()
>   drm/i915: Don't use encoder->new_crtc in intel_lvds_compute_config()
>   drm/i915: Pass an atomic state to modeset_global_resources() functions
>   drm/i915: Check lane sharing between pipes B & C using atomic state
>   drm/i915: Convert intel_pipe_will_have_type() to using atomic state
>   drm/i915: Don't look at staged config crtc when changing DRRS state
>   drm/i915: Remove usage of encoder->new_crtc from clock computations
> 
> Daniel Vetter (1):
>   drm/i915: Add module param to test the load detect code

Except for this one (not in the series) and the fdi lane check code (we
discussed how to change the patch on irc already) all merged to dinq.

Thanks a lot, Daniel
> 
>  drivers/gpu/drm/i915/i915_drv.h      |   5 +-
>  drivers/gpu/drm/i915/i915_params.c   |   8 +-
>  drivers/gpu/drm/i915/intel_crt.c     |   9 +-
>  drivers/gpu/drm/i915/intel_ddi.c     |  24 +-
>  drivers/gpu/drm/i915/intel_display.c | 566 +++++++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_dp.c      |   5 +-
>  drivers/gpu/drm/i915/intel_dp_mst.c  |  18 +-
>  drivers/gpu/drm/i915/intel_drv.h     |  16 +-
>  drivers/gpu/drm/i915/intel_dsi.c     |   1 +
>  drivers/gpu/drm/i915/intel_dvo.c     |   1 +
>  drivers/gpu/drm/i915/intel_hdmi.c    |  22 +-
>  drivers/gpu/drm/i915/intel_lvds.c    |   3 +-
>  drivers/gpu/drm/i915/intel_sdvo.c    |   1 +
>  drivers/gpu/drm/i915/intel_tv.c      |   3 +-
>  14 files changed, 523 insertions(+), 159 deletions(-)
> 
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Check lane sharing between pipes B & C using atomic state

[Ander Conselvan de Oliveira]

Makes that code atomic ready.

v2: Acquire crtc_state for the "other" pipe only when needed. (Daniel)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 65 +++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc9a5c1..68fe8b8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5669,65 +5669,78 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
-static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
+static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc *crtc =
-		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
-
-	if (crtc->base.state->enable &&
-	    crtc->config->has_pch_encoder)
-		return crtc->config->fdi_lanes;
+	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
+		return crtc_state->fdi_lanes;
 
 	return 0;
 }
 
-static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
+static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 				     struct intel_crtc_state *pipe_config)
 {
+	struct drm_atomic_state *state = pipe_config->base.state;
+	struct intel_crtc *other_crtc;
+	struct intel_crtc_state *other_crtc_state;
+
 	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
 		      pipe_name(pipe), pipe_config->fdi_lanes);
 	if (pipe_config->fdi_lanes > 4) {
 		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
 			      pipe_name(pipe), pipe_config->fdi_lanes);
-		return false;
+		return -EINVAL;
 	}
 
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		if (pipe_config->fdi_lanes > 2) {
 			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
 				      pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		} else {
-			return true;
+			return 0;
 		}
 	}
 
 	if (INTEL_INFO(dev)->num_pipes == 2)
-		return true;
+		return 0;
 
 	/* Ivybridge 3 pipe is really complicated */
 	switch (pipe) {
 	case PIPE_A:
-		return true;
+		return 0;
 	case PIPE_B:
+		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
+		other_crtc_state =
+			intel_atomic_get_crtc_state(state, other_crtc);
+		if (IS_ERR(other_crtc_state))
+			return PTR_ERR(other_crtc_state);
+
 		if (pipe_config->fdi_lanes > 2 &&
-		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
+		    pipe_required_fdi_lanes(other_crtc_state) > 0) {
 			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
 				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		}
-		return true;
+		return 0;
 	case PIPE_C:
 		if (pipe_config->fdi_lanes > 2) {
 			DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n",
 				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		}
-		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
+
+		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
+		other_crtc_state =
+			intel_atomic_get_crtc_state(state, other_crtc);
+		if (IS_ERR(other_crtc_state))
+			return PTR_ERR(other_crtc_state);
+
+		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
 			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
-			return false;
+			return -EINVAL;
 		}
-		return true;
+		return 0;
 	default:
 		BUG();
 	}
@@ -5739,8 +5752,8 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	int lane, link_bw, fdi_dotclock;
-	bool setup_ok, needs_recompute = false;
+	int lane, link_bw, fdi_dotclock, ret;
+	bool needs_recompute = false;
 
 retry:
 	/* FDI is a binary signal running at ~2.7GHz, encoding
@@ -5762,9 +5775,9 @@ retry:
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
 			       link_bw, &pipe_config->fdi_m_n);
 
-	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
-					    intel_crtc->pipe, pipe_config);
-	if (!setup_ok && pipe_config->pipe_bpp > 6*3) {
+	ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
+				       intel_crtc->pipe, pipe_config);
+	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
 		pipe_config->pipe_bpp -= 2*3;
 		DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to %i\n",
 			      pipe_config->pipe_bpp);
@@ -5777,7 +5790,7 @@ retry:
 	if (needs_recompute)
 		return RETRY;
 
-	return setup_ok ? 0 : -EINVAL;
+	return ret;
 }
 
 static void hsw_compute_ips_config(struct intel_crtc *crtc,
-- 
2.1.0

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

Re: [PATCH] drm/i915: Check lane sharing between pipes B & C using atomic state

[Daniel Vetter]

On Fri, Mar 27, 2015 at 03:00:08PM +0200, Ander Conselvan de Oliveira wrote:
> Makes that code atomic ready.
> 
> v2: Acquire crtc_state for the "other" pipe only when needed. (Daniel)
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 65 +++++++++++++++++++++---------------
>  1 file changed, 39 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dc9a5c1..68fe8b8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5669,65 +5669,78 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
>  	return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> -static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
> +static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_crtc *crtc =
> -		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> -
> -	if (crtc->base.state->enable &&
> -	    crtc->config->has_pch_encoder)
> -		return crtc->config->fdi_lanes;
> +	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
> +		return crtc_state->fdi_lanes;
>  
>  	return 0;
>  }
>  
> -static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
> +static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  				     struct intel_crtc_state *pipe_config)
>  {
> +	struct drm_atomic_state *state = pipe_config->base.state;
> +	struct intel_crtc *other_crtc;
> +	struct intel_crtc_state *other_crtc_state;
> +
>  	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
>  		      pipe_name(pipe), pipe_config->fdi_lanes);
>  	if (pipe_config->fdi_lanes > 4) {
>  		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
>  			      pipe_name(pipe), pipe_config->fdi_lanes);
> -		return false;
> +		return -EINVAL;
>  	}
>  
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		if (pipe_config->fdi_lanes > 2) {
>  			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
>  				      pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		} else {
> -			return true;
> +			return 0;
>  		}
>  	}
>  
>  	if (INTEL_INFO(dev)->num_pipes == 2)
> -		return true;
> +		return 0;
>  
>  	/* Ivybridge 3 pipe is really complicated */
>  	switch (pipe) {
>  	case PIPE_A:
> -		return true;
> +		return 0;
>  	case PIPE_B:

Since we bother with all this trouble I think a

		if (pipe_config->fdi_lanes <= 2)
			return 0;

here before we acquire the other crtc state and then removing the check
below would be really pretty. Just so that we really only acquire the
other states when we need to ;-) lgtm otherwise.
-Daniel

> +		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
> +		other_crtc_state =
> +			intel_atomic_get_crtc_state(state, other_crtc);
> +		if (IS_ERR(other_crtc_state))
> +			return PTR_ERR(other_crtc_state);
> +
>  		if (pipe_config->fdi_lanes > 2 &&
> -		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
> +		    pipe_required_fdi_lanes(other_crtc_state) > 0) {
>  			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		}
> -		return true;
> +		return 0;
>  	case PIPE_C:
>  		if (pipe_config->fdi_lanes > 2) {
>  			DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		}
> -		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
> +
> +		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
> +		other_crtc_state =
> +			intel_atomic_get_crtc_state(state, other_crtc);
> +		if (IS_ERR(other_crtc_state))
> +			return PTR_ERR(other_crtc_state);
> +
> +		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
>  			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
> -			return false;
> +			return -EINVAL;
>  		}
> -		return true;
> +		return 0;
>  	default:
>  		BUG();
>  	}
> @@ -5739,8 +5752,8 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> -	int lane, link_bw, fdi_dotclock;
> -	bool setup_ok, needs_recompute = false;
> +	int lane, link_bw, fdi_dotclock, ret;
> +	bool needs_recompute = false;
>  
>  retry:
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
> @@ -5762,9 +5775,9 @@ retry:
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
>  			       link_bw, &pipe_config->fdi_m_n);
>  
> -	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> -					    intel_crtc->pipe, pipe_config);
> -	if (!setup_ok && pipe_config->pipe_bpp > 6*3) {
> +	ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> +				       intel_crtc->pipe, pipe_config);
> +	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
>  		pipe_config->pipe_bpp -= 2*3;
>  		DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to %i\n",
>  			      pipe_config->pipe_bpp);
> @@ -5777,7 +5790,7 @@ retry:
>  	if (needs_recompute)
>  		return RETRY;
>  
> -	return setup_ok ? 0 : -EINVAL;
> +	return ret;
>  }
>  
>  static void hsw_compute_ips_config(struct intel_crtc *crtc,
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check lane sharing between pipes B & C using atomic state

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6080
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  303/303              303/303
SNB                                  304/304              304/304
IVB                 -2              330/330              328/330
BYT                                  287/287              287/287
HSW                                  361/361              361/361
BDW                                  309/309              309/309
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*IVB  igt@gem_storedw_batches_loop@normal      PASS(6)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
*IVB  igt@gem_pwrite_pread@snooped-copy-performance      PASS(5)      DMESG_WARN(2)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Check lane sharing between pipes B & C using atomic state

[Ander Conselvan de Oliveira]

Makes that code atomic ready.

v2: Acquire crtc_state for the "other" pipe only when needed. (Daniel)

v3: Really only acquire the other state if necessary. (Daniel)
---
 drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75955fe..cfe34c3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5667,65 +5667,80 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
 	return encoder->get_hw_state(encoder, &pipe);
 }
 
-static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
+static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc *crtc =
-		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
-
-	if (crtc->base.state->enable &&
-	    crtc->config->has_pch_encoder)
-		return crtc->config->fdi_lanes;
+	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
+		return crtc_state->fdi_lanes;
 
 	return 0;
 }
 
-static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
+static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
 				     struct intel_crtc_state *pipe_config)
 {
+	struct drm_atomic_state *state = pipe_config->base.state;
+	struct intel_crtc *other_crtc;
+	struct intel_crtc_state *other_crtc_state;
+
 	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
 		      pipe_name(pipe), pipe_config->fdi_lanes);
 	if (pipe_config->fdi_lanes > 4) {
 		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
 			      pipe_name(pipe), pipe_config->fdi_lanes);
-		return false;
+		return -EINVAL;
 	}
 
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
 		if (pipe_config->fdi_lanes > 2) {
 			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
 				      pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		} else {
-			return true;
+			return 0;
 		}
 	}
 
 	if (INTEL_INFO(dev)->num_pipes == 2)
-		return true;
+		return 0;
 
 	/* Ivybridge 3 pipe is really complicated */
 	switch (pipe) {
 	case PIPE_A:
-		return true;
+		return 0;
 	case PIPE_B:
-		if (pipe_config->fdi_lanes > 2 &&
-		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
+		if (pipe_config->fdi_lanes <= 2)
+			return 0;
+
+		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
+		other_crtc_state =
+			intel_atomic_get_crtc_state(state, other_crtc);
+		if (IS_ERR(other_crtc_state))
+			return PTR_ERR(other_crtc_state);
+
+		if (pipe_required_fdi_lanes(other_crtc_state) > 0) {
 			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
 				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		}
-		return true;
+		return 0;
 	case PIPE_C:
 		if (pipe_config->fdi_lanes > 2) {
 			DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n",
 				      pipe_name(pipe), pipe_config->fdi_lanes);
-			return false;
+			return -EINVAL;
 		}
-		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
+
+		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
+		other_crtc_state =
+			intel_atomic_get_crtc_state(state, other_crtc);
+		if (IS_ERR(other_crtc_state))
+			return PTR_ERR(other_crtc_state);
+
+		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
 			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
-			return false;
+			return -EINVAL;
 		}
-		return true;
+		return 0;
 	default:
 		BUG();
 	}
@@ -5737,8 +5752,8 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	int lane, link_bw, fdi_dotclock;
-	bool setup_ok, needs_recompute = false;
+	int lane, link_bw, fdi_dotclock, ret;
+	bool needs_recompute = false;
 
 retry:
 	/* FDI is a binary signal running at ~2.7GHz, encoding
@@ -5760,9 +5775,9 @@ retry:
 	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
 			       link_bw, &pipe_config->fdi_m_n);
 
-	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
-					    intel_crtc->pipe, pipe_config);
-	if (!setup_ok && pipe_config->pipe_bpp > 6*3) {
+	ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
+				       intel_crtc->pipe, pipe_config);
+	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
 		pipe_config->pipe_bpp -= 2*3;
 		DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to %i\n",
 			      pipe_config->pipe_bpp);
@@ -5775,7 +5790,7 @@ retry:
 	if (needs_recompute)
 		return RETRY;
 
-	return setup_ok ? 0 : -EINVAL;
+	return ret;
 }
 
 static void hsw_compute_ips_config(struct intel_crtc *crtc,
-- 
2.1.0

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

Re: [PATCH] drm/i915: Check lane sharing between pipes B & C using atomic state

[Daniel Vetter]

On Mon, Mar 30, 2015 at 08:33:12AM +0300, Ander Conselvan de Oliveira wrote:
> Makes that code atomic ready.
> 
> v2: Acquire crtc_state for the "other" pipe only when needed. (Daniel)
> 
> v3: Really only acquire the other state if necessary. (Daniel)

Missing sob added and queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 69 ++++++++++++++++++++++--------------
>  1 file changed, 42 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 75955fe..cfe34c3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5667,65 +5667,80 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
>  	return encoder->get_hw_state(encoder, &pipe);
>  }
>  
> -static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
> +static int pipe_required_fdi_lanes(struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_crtc *crtc =
> -		to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> -
> -	if (crtc->base.state->enable &&
> -	    crtc->config->has_pch_encoder)
> -		return crtc->config->fdi_lanes;
> +	if (crtc_state->base.enable && crtc_state->has_pch_encoder)
> +		return crtc_state->fdi_lanes;
>  
>  	return 0;
>  }
>  
> -static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
> +static int ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
>  				     struct intel_crtc_state *pipe_config)
>  {
> +	struct drm_atomic_state *state = pipe_config->base.state;
> +	struct intel_crtc *other_crtc;
> +	struct intel_crtc_state *other_crtc_state;
> +
>  	DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
>  		      pipe_name(pipe), pipe_config->fdi_lanes);
>  	if (pipe_config->fdi_lanes > 4) {
>  		DRM_DEBUG_KMS("invalid fdi lane config on pipe %c: %i lanes\n",
>  			      pipe_name(pipe), pipe_config->fdi_lanes);
> -		return false;
> +		return -EINVAL;
>  	}
>  
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
>  		if (pipe_config->fdi_lanes > 2) {
>  			DRM_DEBUG_KMS("only 2 lanes on haswell, required: %i lanes\n",
>  				      pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		} else {
> -			return true;
> +			return 0;
>  		}
>  	}
>  
>  	if (INTEL_INFO(dev)->num_pipes == 2)
> -		return true;
> +		return 0;
>  
>  	/* Ivybridge 3 pipe is really complicated */
>  	switch (pipe) {
>  	case PIPE_A:
> -		return true;
> +		return 0;
>  	case PIPE_B:
> -		if (pipe_config->fdi_lanes > 2 &&
> -		    pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
> +		if (pipe_config->fdi_lanes <= 2)
> +			return 0;
> +
> +		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_C));
> +		other_crtc_state =
> +			intel_atomic_get_crtc_state(state, other_crtc);
> +		if (IS_ERR(other_crtc_state))
> +			return PTR_ERR(other_crtc_state);
> +
> +		if (pipe_required_fdi_lanes(other_crtc_state) > 0) {
>  			DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %c: %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		}
> -		return true;
> +		return 0;
>  	case PIPE_C:
>  		if (pipe_config->fdi_lanes > 2) {
>  			DRM_DEBUG_KMS("only 2 lanes on pipe %c: required %i lanes\n",
>  				      pipe_name(pipe), pipe_config->fdi_lanes);
> -			return false;
> +			return -EINVAL;
>  		}
> -		if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
> +
> +		other_crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, PIPE_B));
> +		other_crtc_state =
> +			intel_atomic_get_crtc_state(state, other_crtc);
> +		if (IS_ERR(other_crtc_state))
> +			return PTR_ERR(other_crtc_state);
> +
> +		if (pipe_required_fdi_lanes(other_crtc_state) > 2) {
>  			DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n");
> -			return false;
> +			return -EINVAL;
>  		}
> -		return true;
> +		return 0;
>  	default:
>  		BUG();
>  	}
> @@ -5737,8 +5752,8 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
> -	int lane, link_bw, fdi_dotclock;
> -	bool setup_ok, needs_recompute = false;
> +	int lane, link_bw, fdi_dotclock, ret;
> +	bool needs_recompute = false;
>  
>  retry:
>  	/* FDI is a binary signal running at ~2.7GHz, encoding
> @@ -5760,9 +5775,9 @@ retry:
>  	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
>  			       link_bw, &pipe_config->fdi_m_n);
>  
> -	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> -					    intel_crtc->pipe, pipe_config);
> -	if (!setup_ok && pipe_config->pipe_bpp > 6*3) {
> +	ret = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> +				       intel_crtc->pipe, pipe_config);
> +	if (ret == -EINVAL && pipe_config->pipe_bpp > 6*3) {
>  		pipe_config->pipe_bpp -= 2*3;
>  		DRM_DEBUG_KMS("fdi link bw constraint, reducing pipe bpp to %i\n",
>  			      pipe_config->pipe_bpp);
> @@ -5775,7 +5790,7 @@ retry:
>  	if (needs_recompute)
>  		return RETRY;
>  
> -	return setup_ok ? 0 : -EINVAL;
> +	return ret;
>  }
>  
>  static void hsw_compute_ips_config(struct intel_crtc *crtc,
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Check lane sharing between pipes B & C using atomic state

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6087
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              270/270              268/270
ILK                                  303/303              303/303
SNB                                  304/304              304/304
IVB                                  337/337              337/337
BYT                                  287/287              287/287
HSW                                  361/361              361/361
BDW                                  309/309              309/309
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(1)PASS(2)      CRASH(1)PASS(1)
*PNV  igt@gem_tiled_pread_pwrite      PASS(3)      FAIL(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] Enable dithering for intel VCH DVO

[Thomas Richter]

[-- Attachment #1: Type: text/plain, Size: 233 bytes --]

Hi Daniel, hi Ville,

did you get the attached patch? This enables dithering for the iVCH DVO
chip and improves image quality for 24 pipes on 18bpp displays greatly.

Thanks for reviewing and considering this patch.

Thomas Richter


[-- Attachment #2: 0001-Enabled-dithering-in-the-intel-VCH-DVO-for-18bpp-pip.patch --]
[-- Type: text/x-patch, Size: 2612 bytes --]

>From 3d0b1a15302aa704c7cf4ebbf7c2b8a1566b9beb Mon Sep 17 00:00:00 2001
From: Thomas Richter <thor@math.tu-berlin.de>
Date: Sat, 28 Mar 2015 10:57:46 +0100
Subject: [PATCH 1/1] Enabled dithering in the intel VCH DVO for 18bpp
 pipelines.


Signed-off-by: Thomas Richter <thor@math.tu-berlin.de>
---
 drivers/gpu/drm/i915/dvo_ivch.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c
index 0f2587f..89b08a8 100644
--- a/drivers/gpu/drm/i915/dvo_ivch.c
+++ b/drivers/gpu/drm/i915/dvo_ivch.c
@@ -23,6 +23,9 @@
  * Authors:
  *    Eric Anholt <eric@anholt.net>
  *
+ * Minor modifications (Dithering enable):
+ *    Thomas Richter <thor@math.tu-berlin.de>
+ *
  */
 
 #include "dvo.h"
@@ -59,6 +62,8 @@
 # define VR01_DVO_BYPASS_ENABLE		(1 << 1)
 /** Enables the DVO clock */
 # define VR01_DVO_ENABLE		(1 << 0)
+/** Enable dithering for 18bpp panels. Not documented. */
+# define VR01_DITHER_ENABLE             (1 << 4)
 
 /*
  * LCD Interface Format
@@ -74,6 +79,8 @@
 # define VR10_INTERFACE_2X18		(2 << 2)
 /** Enables 2x24-bit LVDS output */
 # define VR10_INTERFACE_2X24		(3 << 2)
+/** Mask that defines the depth of the pipeline */
+# define VR10_INTERFACE_DEPTH_MASK      (3 << 2)
 
 /*
  * VR20 LCD Horizontal Display Size
@@ -342,9 +349,15 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
 			  struct drm_display_mode *adjusted_mode)
 {
 	uint16_t vr40 = 0;
-	uint16_t vr01;
+	uint16_t vr01 = 0;
+	uint16_t vr10;
+
+	ivch_read(dvo, VR10, &vr10);
+	/* Enable dithering for 18 bpp pipelines */
+	vr10 &= VR10_INTERFACE_DEPTH_MASK;
+	if (vr10 == VR10_INTERFACE_2X18 || vr10 == VR10_INTERFACE_1X18)
+		vr01 = VR01_DITHER_ENABLE;
 
-	vr01 = 0;
 	vr40 = (VR40_STALL_ENABLE | VR40_VERTICAL_INTERP_ENABLE |
 		VR40_HORIZONTAL_INTERP_ENABLE);
 
@@ -353,7 +366,7 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
 		uint16_t x_ratio, y_ratio;
 
 		vr01 |= VR01_PANEL_FIT_ENABLE;
-		vr40 |= VR40_CLOCK_GATING_ENABLE;
+		vr40 |= VR40_CLOCK_GATING_ENABLE | VR40_ENHANCED_PANEL_FITTING;
 		x_ratio = (((mode->hdisplay - 1) << 16) /
 			   (adjusted_mode->hdisplay - 1)) >> 2;
 		y_ratio = (((mode->vdisplay - 1) << 16) /
@@ -380,6 +393,8 @@ static void ivch_dump_regs(struct intel_dvo_device *dvo)
 	DRM_DEBUG_KMS("VR00: 0x%04x\n", val);
 	ivch_read(dvo, VR01, &val);
 	DRM_DEBUG_KMS("VR01: 0x%04x\n", val);
+	ivch_read(dvo, VR10, &val);
+	DRM_DEBUG_KMS("VR10: 0x%04x\n", val);
 	ivch_read(dvo, VR30, &val);
 	DRM_DEBUG_KMS("VR30: 0x%04x\n", val);
 	ivch_read(dvo, VR40, &val);
-- 
1.7.10.4


[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] Enable dithering for intel VCH DVO

[Daniel Vetter]

On Mon, Mar 30, 2015 at 10:18:53AM +0200, Thomas Richter wrote:
> Hi Daniel, hi Ville,
> 
> did you get the attached patch? This enables dithering for the iVCH DVO
> chip and improves image quality for 24 pipes on 18bpp displays greatly.
> 
> Thanks for reviewing and considering this patch.

Yeah that one worked better. Queued for -next, thanks for the patch.
-Daniel

> 
> Thomas Richter
> 

> From 3d0b1a15302aa704c7cf4ebbf7c2b8a1566b9beb Mon Sep 17 00:00:00 2001
> From: Thomas Richter <thor@math.tu-berlin.de>
> Date: Sat, 28 Mar 2015 10:57:46 +0100
> Subject: [PATCH 1/1] Enabled dithering in the intel VCH DVO for 18bpp
>  pipelines.
> 
> 
> Signed-off-by: Thomas Richter <thor@math.tu-berlin.de>
> ---
>  drivers/gpu/drm/i915/dvo_ivch.c |   21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c
> index 0f2587f..89b08a8 100644
> --- a/drivers/gpu/drm/i915/dvo_ivch.c
> +++ b/drivers/gpu/drm/i915/dvo_ivch.c
> @@ -23,6 +23,9 @@
>   * Authors:
>   *    Eric Anholt <eric@anholt.net>
>   *
> + * Minor modifications (Dithering enable):
> + *    Thomas Richter <thor@math.tu-berlin.de>
> + *
>   */
>  
>  #include "dvo.h"
> @@ -59,6 +62,8 @@
>  # define VR01_DVO_BYPASS_ENABLE		(1 << 1)
>  /** Enables the DVO clock */
>  # define VR01_DVO_ENABLE		(1 << 0)
> +/** Enable dithering for 18bpp panels. Not documented. */
> +# define VR01_DITHER_ENABLE             (1 << 4)
>  
>  /*
>   * LCD Interface Format
> @@ -74,6 +79,8 @@
>  # define VR10_INTERFACE_2X18		(2 << 2)
>  /** Enables 2x24-bit LVDS output */
>  # define VR10_INTERFACE_2X24		(3 << 2)
> +/** Mask that defines the depth of the pipeline */
> +# define VR10_INTERFACE_DEPTH_MASK      (3 << 2)
>  
>  /*
>   * VR20 LCD Horizontal Display Size
> @@ -342,9 +349,15 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
>  			  struct drm_display_mode *adjusted_mode)
>  {
>  	uint16_t vr40 = 0;
> -	uint16_t vr01;
> +	uint16_t vr01 = 0;
> +	uint16_t vr10;
> +
> +	ivch_read(dvo, VR10, &vr10);
> +	/* Enable dithering for 18 bpp pipelines */
> +	vr10 &= VR10_INTERFACE_DEPTH_MASK;
> +	if (vr10 == VR10_INTERFACE_2X18 || vr10 == VR10_INTERFACE_1X18)
> +		vr01 = VR01_DITHER_ENABLE;
>  
> -	vr01 = 0;
>  	vr40 = (VR40_STALL_ENABLE | VR40_VERTICAL_INTERP_ENABLE |
>  		VR40_HORIZONTAL_INTERP_ENABLE);
>  
> @@ -353,7 +366,7 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
>  		uint16_t x_ratio, y_ratio;
>  
>  		vr01 |= VR01_PANEL_FIT_ENABLE;
> -		vr40 |= VR40_CLOCK_GATING_ENABLE;
> +		vr40 |= VR40_CLOCK_GATING_ENABLE | VR40_ENHANCED_PANEL_FITTING;
>  		x_ratio = (((mode->hdisplay - 1) << 16) /
>  			   (adjusted_mode->hdisplay - 1)) >> 2;
>  		y_ratio = (((mode->vdisplay - 1) << 16) /
> @@ -380,6 +393,8 @@ static void ivch_dump_regs(struct intel_dvo_device *dvo)
>  	DRM_DEBUG_KMS("VR00: 0x%04x\n", val);
>  	ivch_read(dvo, VR01, &val);
>  	DRM_DEBUG_KMS("VR01: 0x%04x\n", val);
> +	ivch_read(dvo, VR10, &val);
> +	DRM_DEBUG_KMS("VR10: 0x%04x\n", val);
>  	ivch_read(dvo, VR30, &val);
>  	DRM_DEBUG_KMS("VR30: 0x%04x\n", val);
>  	ivch_read(dvo, VR40, &val);
> -- 
> 1.7.10.4
> 


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] Enable dithering for intel VCH DVO

[Jani Nikula]

On Mon, 30 Mar 2015, Thomas Richter <thor@math.tu-berlin.de> wrote:
> Hi Daniel, hi Ville,
>
> did you get the attached patch? This enables dithering for the iVCH DVO
> chip and improves image quality for 24 pipes on 18bpp displays greatly.
>
> Thanks for reviewing and considering this patch.

Thomas, I've observed many times in the past there's something funny
with how you reply to mails on intel-gfx, and I've told you before. Like
this one has headers:

References: <20150327140357.GV23521@phenom.ffwll.local>
 <1427693592-12941-1-git-send-email-ander.conselvan.de.oliveira@intel.com>
 <28056_1427699754_5518F829_28056_1298_1_20150330071734.GH23521@phenom.ffwll.local>
In-Reply-To: <28056_1427699754_5518F829_28056_1298_1_20150330071734.GH23521@phenom.ffwll.local>

The first two referenced Message-IDs are valid for messages in Ander's
patch series [1], while the third one doesn't even exist *unless* the
apparently bogus prefix "28056_1427699754_5518F829_28056_1298_1_" is
removed. And even that then is in Ander's patch series. Your message has
nothing to do with that thread.

Please check your setup. Please don't keep replying to unrelated
threads. Note that just changing the subject does not make your reply
become a new, independent thread.


Much appreciated,
Jani.


[1] http://mid.gmane.org/1426861099-28445-1-git-send-email-ander.conselvan.de.oliveira@intel.com


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx