[PATCH 5/6] drm/i915: update pipe size at set_config time

[PATCH 5/6] drm/i915: update pipe size at set_config time

[Jesse Barnes]

This only affects the fastboot path as-is.  In that case, we simply need
to make sure that we update the pipe size at the first mode set.  Rather
than putting it off until we decide to flip (if indeed we do end up
flipping), update the pipe size as appropriate a bit earlier in the
set_config call.

This sets us up for better pipe tracking in later patches.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 235933a..10468a7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2902,8 +2902,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return ret;
 	}
 
-	intel_update_pipe_size(intel_crtc);
-
 	dev_priv->display.update_primary_plane(crtc, fb, x, y);
 
 	if (intel_crtc->active)
@@ -11488,6 +11486,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	    to_intel_crtc(set->crtc)->config.has_infoframe)
 		config->mode_changed = true;
 
+	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
 	if (config->mode_changed) {
 		ret = intel_set_mode_pipes(set->crtc, set->mode,
-- 
1.9.1

[PATCH 5/6] drm/i915: update pipe size at set_config time

[Jesse Barnes]

This only affects the fastboot path as-is.  In that case, we simply need
to make sure that we update the pipe size at the first mode set.  Rather
than putting it off until we decide to flip (if indeed we do end up
flipping), update the pipe size as appropriate a bit earlier in the
set_config call.

This sets us up for better pipe tracking in later patches.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2199a9a..470cdac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2902,8 +2902,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return ret;
 	}
 
-	intel_update_pipe_size(intel_crtc);
-
 	dev_priv->display.update_primary_plane(crtc, fb, x, y);
 
 	if (intel_crtc->active)
@@ -11491,6 +11489,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	    to_intel_crtc(set->crtc)->config.has_infoframe)
 		config->mode_changed = true;
 
+	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
 	if (config->mode_changed) {
 		ret = intel_set_mode_pipes(set->crtc, set->mode,
-- 
1.9.1

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

[PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode v3

[Jesse Barnes]

This allows us to calculate the full pipe config before we do any mode
setting work.

v2:
  - clarify comments about global vs. per-crtc mode set (Ander)
  - clean up unnecessary pipe_config = NULL setting (Ander)
v3:
  - fix pipe_config handling (alloc in compute_config, free in set_mode) (Jesse)
  - fix arg order in set_mode (Jesse)
  - fix failure path of set_config (Ander)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 105 ++++++++++++++++++++++++-----------
 1 file changed, 74 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a9df85f..a3ebab8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10656,45 +10656,60 @@ static void update_scanline_offset(struct intel_crtc *crtc)
 		crtc->scanline_offset = 1;
 }
 
+static struct intel_crtc_config *
+intel_modeset_compute_config(struct drm_crtc *crtc,
+			     struct drm_display_mode *mode,
+			     struct drm_framebuffer *fb,
+			     unsigned *modeset_pipes,
+			     unsigned *prepare_pipes,
+			     unsigned *disable_pipes)
+{
+	struct intel_crtc_config *pipe_config = NULL;
+
+	intel_modeset_affected_pipes(crtc, modeset_pipes,
+				     prepare_pipes, disable_pipes);
+
+	if ((*modeset_pipes) == 0)
+		goto out;
+
+	/*
+	 * Note this needs changes when we start tracking multiple modes
+	 * and crtcs.  At that point we'll need to compute the whole config
+	 * (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;
+	}
+	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+			       "[modeset]");
+	to_intel_crtc(crtc)->new_config = pipe_config;
+
+out:
+	return pipe_config;
+}
+
 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 intel_crtc_config *pipe_config,
+			    unsigned modeset_pipes,
+			    unsigned prepare_pipes,
+			    unsigned disable_pipes)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_display_mode *saved_mode;
-	struct intel_crtc_config *pipe_config = NULL;
 	struct intel_crtc *intel_crtc;
-	unsigned disable_pipes, prepare_pipes, modeset_pipes;
 	int ret = 0;
 
 	saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
 	if (!saved_mode)
 		return -ENOMEM;
 
-	intel_modeset_affected_pipes(crtc, &modeset_pipes,
-				     &prepare_pipes, &disable_pipes);
-
 	*saved_mode = crtc->mode;
 
-	/* Hack: Because we don't (yet) support global modeset on multiple
-	 * crtcs, we don't keep track of the new mode for more than one crtc.
-	 * Hence simply check whether any bit is set in modeset_pipes in all the
-	 * pieces of code that are not yet converted to deal with mutliple crtcs
-	 * changing their mode at the same time. */
-	if (modeset_pipes) {
-		pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
-		if (IS_ERR(pipe_config)) {
-			ret = PTR_ERR(pipe_config);
-			pipe_config = NULL;
-
-			goto out;
-		}
-		intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
-				       "[modeset]");
-		to_intel_crtc(crtc)->new_config = pipe_config;
-	}
-
 	/*
 	 * See if the config requires any additional preparation, e.g.
 	 * to adjust global state with pipes off.  We need to do this
@@ -10719,6 +10734,10 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 
 	/* crtc->mode is already used by the ->mode_set callbacks, hence we need
 	 * to set it here already despite that we pass it down the callchain.
+	 *
+	 * Note we'll need to fix this up when we start tracking multiple
+	 * pipes; here we assume a single modeset_pipe and only track the
+	 * single crtc and mode.
 	 */
 	if (modeset_pipes) {
 		crtc->mode = *mode;
@@ -10787,19 +10806,23 @@ done:
 	if (ret && crtc->enabled)
 		crtc->mode = *saved_mode;
 
-out:
 	kfree(pipe_config);
 	kfree(saved_mode);
 	return ret;
 }
 
-static int intel_set_mode(struct drm_crtc *crtc,
-			  struct drm_display_mode *mode,
-			  int x, int y, struct drm_framebuffer *fb)
+static int intel_set_mode_pipes(struct drm_crtc *crtc,
+				struct drm_display_mode *mode,
+				int x, int y, struct drm_framebuffer *fb,
+				struct intel_crtc_config *pipe_config,
+				unsigned modeset_pipes,
+				unsigned prepare_pipes,
+				unsigned disable_pipes)
 {
 	int ret;
 
-	ret = __intel_set_mode(crtc, mode, x, y, fb);
+	ret = __intel_set_mode(crtc, mode, x, y, fb, pipe_config, modeset_pipes,
+			       prepare_pipes, disable_pipes);
 
 	if (ret == 0)
 		intel_modeset_check_state(crtc->dev);
@@ -10807,6 +10830,26 @@ static int intel_set_mode(struct drm_crtc *crtc,
 	return ret;
 }
 
+static int intel_set_mode(struct drm_crtc *crtc,
+			  struct drm_display_mode *mode,
+			  int x, int y, struct drm_framebuffer *fb)
+{
+	struct intel_crtc_config *pipe_config;
+	unsigned modeset_pipes, prepare_pipes, disable_pipes;
+
+	pipe_config = intel_modeset_compute_config(crtc, mode, fb,
+						   &modeset_pipes,
+						   &prepare_pipes,
+						   &disable_pipes);
+
+	if (IS_ERR(pipe_config))
+		return PTR_ERR(pipe_config);
+
+	return intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
+				    modeset_pipes, prepare_pipes,
+				    disable_pipes);
+}
+
 void intel_crtc_restore_mode(struct drm_crtc *crtc)
 {
 	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb);
@@ -13156,8 +13199,8 @@ 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_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
+				       crtc->primary->fb);
 		}
 	} else {
 		intel_modeset_update_staged_output_state(dev);
-- 
1.9.1

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

[PATCH 2/6] drm/i915: use compute_config in set_config v3

[Jesse Barnes]

This will allow us to consult more info before deciding whether to flip
or do a full mode set.

v2:
  - don't use uninitialized or incorrect pipe masks in set_config
    failure path (Ander)
v3:
  - fixup for pipe_config changes in compute_config (Jesse)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a3ebab8..cb96f11 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	struct drm_device *dev;
 	struct drm_mode_set save_set;
 	struct intel_set_config *config;
+	struct intel_crtc_config *pipe_config;
+	unsigned modeset_pipes, prepare_pipes, disable_pipes;
 	int ret;
 
 	BUG_ON(!set);
@@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (ret)
 		goto fail;
 
+	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
+						   set->fb,
+						   &modeset_pipes,
+						   &prepare_pipes,
+						   &disable_pipes);
+	if (IS_ERR(pipe_config))
+		goto fail;
+
+	/* set_mode will free it in the mode_changed case */
+	if (!config->mode_changed)
+		kfree(pipe_config);
+
 	if (config->mode_changed) {
-		ret = intel_set_mode(set->crtc, set->mode,
-				     set->x, set->y, set->fb);
+		ret = intel_set_mode_pipes(set->crtc, set->mode,
+					   set->x, set->y, set->fb, pipe_config,
+					   modeset_pipes, prepare_pipes,
+					   disable_pipes);
 	} else if (config->fb_changed) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
@@ -13198,6 +13214,20 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 		for_each_pipe(dev_priv, pipe) {
 			struct drm_crtc *crtc =
 				dev_priv->pipe_to_crtc_mapping[pipe];
+			struct intel_crtc_config *pipe_config;
+			unsigned modeset_pipes, prepare_pipes, disable_pipes;
+
+			pipe_config = intel_modeset_compute_config(crtc,
+								   &crtc->mode,
+								   crtc->primary->fb,
+								   &modeset_pipes,
+								   &prepare_pipes,
+								   &disable_pipes);
+			if (IS_ERR(pipe_config)) {
+				DRM_DEBUG_KMS("prepare failed\n");
+				kfree(pipe_config);
+				goto out;
+			}
 
 			intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
 				       crtc->primary->fb);
@@ -13205,7 +13235,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 	} else {
 		intel_modeset_update_staged_output_state(dev);
 	}
-
+out:
 	intel_modeset_check_state(dev);
 }
 
-- 
1.9.1

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

[PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Jesse Barnes]

This is useful for checking things later.

v2:
  - fix hsw infoframe enabled check (Ander)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_drv.h  |  4 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 62 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d53ac23..8aa80e1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -292,6 +292,9 @@ struct intel_crtc_config {
 	 * between pch encoders and cpu encoders. */
 	bool has_pch_encoder;
 
+	/* Are we sending infoframes on the attached port */
+	bool has_infoframe;
+
 	/* CPU Transcoder for the pipe. Currently this can only differ from the
 	 * pipe on Haswell (where we have a special eDP transcoder). */
 	enum transcoder cpu_transcoder;
@@ -543,6 +546,7 @@ struct intel_hdmi {
 	void (*set_infoframes)(struct drm_encoder *encoder,
 			       bool enable,
 			       struct drm_display_mode *adjusted_mode);
+	bool (*infoframe_enabled)(struct drm_encoder *encoder);
 };
 
 struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 07b5ebd..994237a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -166,6 +166,15 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
 	POSTING_READ(VIDEO_DIP_CTL);
 }
 
+static bool g4x_infoframe_enabled(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 val = I915_READ(VIDEO_DIP_CTL);
+
+	return val & VIDEO_DIP_ENABLE;
+}
+
 static void ibx_write_infoframe(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len)
@@ -204,6 +213,17 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
 	POSTING_READ(reg);
 }
 
+static bool ibx_infoframe_enabled(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+	u32 val = I915_READ(reg);
+
+	return val & VIDEO_DIP_ENABLE;
+}
+
 static void cpt_write_infoframe(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len)
@@ -245,6 +265,17 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
 	POSTING_READ(reg);
 }
 
+static bool cpt_infoframe_enabled(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+	u32 val = I915_READ(reg);
+
+	return val & VIDEO_DIP_ENABLE;
+}
+
 static void vlv_write_infoframe(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len)
@@ -283,6 +314,17 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
 	POSTING_READ(reg);
 }
 
+static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+	int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
+	u32 val = I915_READ(reg);
+
+	return val & VIDEO_DIP_ENABLE;
+}
+
 static void hsw_write_infoframe(struct drm_encoder *encoder,
 				enum hdmi_infoframe_type type,
 				const void *frame, ssize_t len)
@@ -320,6 +362,18 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
 	POSTING_READ(ctl_reg);
 }
 
+static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
+	u32 val = I915_READ(ctl_reg);
+
+	return val & (VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
+		      VIDEO_DIP_ENABLE_VS_HSW);
+}
+
 /*
  * The data we write to the DIP data buffer registers is 1 byte bigger than the
  * HDMI infoframe size because of an ECC/reserved byte at position 3 (starting
@@ -732,6 +786,9 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	if (tmp & HDMI_MODE_SELECT_HDMI)
 		pipe_config->has_hdmi_sink = true;
 
+	if (intel_hdmi->infoframe_enabled(&encoder->base))
+		pipe_config->has_infoframe = true;
+
 	if (tmp & SDVO_AUDIO_ENABLE)
 		pipe_config->has_audio = true;
 
@@ -1616,18 +1673,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	if (IS_VALLEYVIEW(dev)) {
 		intel_hdmi->write_infoframe = vlv_write_infoframe;
 		intel_hdmi->set_infoframes = vlv_set_infoframes;
+		intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
 	} else if (IS_G4X(dev)) {
 		intel_hdmi->write_infoframe = g4x_write_infoframe;
 		intel_hdmi->set_infoframes = g4x_set_infoframes;
+		intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
 	} else if (HAS_DDI(dev)) {
 		intel_hdmi->write_infoframe = hsw_write_infoframe;
 		intel_hdmi->set_infoframes = hsw_set_infoframes;
+		intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
 	} else if (HAS_PCH_IBX(dev)) {
 		intel_hdmi->write_infoframe = ibx_write_infoframe;
 		intel_hdmi->set_infoframes = ibx_set_infoframes;
+		intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
 	} else {
 		intel_hdmi->write_infoframe = cpt_write_infoframe;
 		intel_hdmi->set_infoframes = cpt_set_infoframes;
+		intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
 	}
 
 	if (HAS_DDI(dev))
-- 
1.9.1

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

[PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jesse Barnes]

If these change (e.g. after a modeset following a fastboot), we need to
do a full mode set.

v2:
  - put under pipe_config check so we don't deref a null state (Jesse)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb96f11..c86eee6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 						   &modeset_pipes,
 						   &prepare_pipes,
 						   &disable_pipes);
-	if (IS_ERR(pipe_config))
+	if (IS_ERR(pipe_config)) {
 		goto fail;
+	} else if (pipe_config) {
+		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
+		    to_intel_crtc(set->crtc)->config.has_audio)
+			config->mode_changed = true;
+
+		/* Force mode sets for any infoframe stuff */
+		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
+		    to_intel_crtc(set->crtc)->config.has_infoframe)
+			config->mode_changed = true;
+	}
 
 	/* set_mode will free it in the mode_changed case */
 	if (!config->mode_changed)
-- 
1.9.1

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

[PATCH 5/6] drm/i915: update pipe size at set_config time

[Jesse Barnes]

This only affects the fastboot path as-is.  In that case, we simply need
to make sure that we update the pipe size at the first mode set.  Rather
than putting it off until we decide to flip (if indeed we do end up
flipping), update the pipe size as appropriate a bit earlier in the
set_config call.

This sets us up for better pipe tracking in later patches.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c86eee6..3f1515d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2906,8 +2906,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return ret;
 	}
 
-	intel_update_pipe_size(intel_crtc);
-
 	dev_priv->display.update_primary_plane(crtc, fb, x, y);
 
 	if (intel_crtc->active)
@@ -11247,6 +11245,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (!config->mode_changed)
 		kfree(pipe_config);
 
+	intel_update_pipe_size(to_intel_crtc(set->crtc));
+
 	if (config->mode_changed) {
 		ret = intel_set_mode_pipes(set->crtc, set->mode,
 					   set->x, set->y, set->fb, pipe_config,
-- 
1.9.1

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

[PATCH 6/6] drm/i915: calculate pfit changes in set_config v2

[Jesse Barnes]

This should allow us to avoid mode sets for some panel fitter config
changes.

v2:
  - fixup pfit comment (Ander)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3f1515d..49281d7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
 		return;
 
 	/*
-	 * Update pipe size and adjust fitter if needed: the reason for this is
-	 * that in compute_mode_changes we check the native mode (not the pfit
-	 * mode) to see if we can flip rather than do a full mode set. In the
-	 * fastboot case, we'll flip, but if we don't update the pipesrc and
-	 * pfit state, we'll end up with a big fb scanned out into the wrong
-	 * sized surface.
-	 *
-	 * To fix this properly, we need to hoist the checks up into
-	 * compute_mode_changes (or above), check the actual pfit state and
-	 * whether the platform allows pfit disable with pipe active, and only
-	 * then update the pipesrc and pfit state, even on the flip path.
+	 * See intel_pfit_changed for info on when we're allowed to
+	 * do this w/o a pipe shutdown.
 	 */
 
 	adjusted_mode = &crtc->config.adjusted_mode;
@@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
 	crtc->new_config = NULL;
 }
 
+/* Do we need a mode set due to pfit changes? */
+static bool intel_pfit_changed(struct drm_device *dev,
+			       struct intel_crtc_config *new_config,
+			       struct intel_crtc_config *cur_config)
+{
+	bool ret = false;
+
+	if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
+		/*
+		 * On PCH platforms we can disable pfit w/o a pipe shutdown,
+		 * otherwise we'll need a mode set.
+		 */
+		if (new_config->pch_pfit.enabled &&
+		    cur_config->pch_pfit.enabled)
+			ret = false;
+		else if (new_config->pch_pfit.enabled &&
+			 !cur_config->pch_pfit.enabled)
+			ret = true;
+		else if (!new_config->pch_pfit.enabled &&
+			 cur_config->pch_pfit.enabled)
+			ret = false;
+		else if (!new_config->pch_pfit.enabled &&
+			 !cur_config->pch_pfit.enabled)
+			ret = false;
+	} else {
+		bool new_enabled, old_enabled;
+
+		new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
+		old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
+
+		/* 9xx only needs a shutdown to disable pfit */
+		if (new_enabled && old_enabled)
+			ret = false;
+		else if (new_enabled && !old_enabled)
+			ret = false;
+		else if (!new_enabled && old_enabled)
+			ret = true;
+		else if (!new_enabled && !old_enabled)
+			ret = false;
+	}
+
+	return ret;
+}
+
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
 	struct drm_device *dev;
@@ -11239,6 +11274,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
 		    to_intel_crtc(set->crtc)->config.has_infoframe)
 			config->mode_changed = true;
+
+		if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config,
+				       &to_intel_crtc(set->crtc)->config))
+			config->mode_changed = true;
 	}
 
 	/* set_mode will free it in the mode_changed case */
-- 
1.9.1

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

Re: [PATCH 6/6] drm/i915: calculate pfit changes in

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=348/348->348/348
PNV: pass/total=324/328->324/328
ILK: pass/total=330/330->330/330
IVB: pass/total=488/497->488/497
SNB: pass/total=515/525->516/525
HSW: pass/total=497/505->497/505
BDW: pass/total=430/435->429/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
PNV: Intel_gpu_tools, igt_drm_vma_limiter_cached, PASS(4, M23M25) -> DMESG_WARN(1, M25)PASS(3, M25)
PNV: Intel_gpu_tools, igt_gem_linear_blits_normal, CRASH(1, M23)DMESG_WARN(1, M24)PASS(8, M24M25) -> PASS(4, M25)
PNV: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, DMESG_WARN(1, M23)TIMEOUT(3, M25) -> DMESG_WARN(3, M25)TIMEOUT(1, M25)
IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(1, M4)PASS(9, M4M34) -> TIMEOUT(1, M34)PASS(3, M34)
IVB: Intel_gpu_tools, igt_kms_pipe_crc_basic_hang-read-crc-pipe-B, PASS(4, M4M34) -> TIMEOUT(1, M34)PASS(3, M34)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-offscreen, DMESG_WARN(1, M35)PASS(6, M35) -> PASS(4, M35)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(1, M35)PASS(9, M35M22) -> PASS(4, M35)
SNB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(3, M35)PASS(1, M35) -> TIMEOUT(1, M35)PASS(3, M35)
BDW: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(3, M30)PASS(1, M42) -> TIMEOUT(1, M30)PASS(3, M30)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode v3

[Chris Wilson]

On Wed, Nov 05, 2014 at 02:26:06PM -0800, Jesse Barnes wrote:
> +static int intel_set_mode(struct drm_crtc *crtc,
> +			  struct drm_display_mode *mode,
> +			  int x, int y, struct drm_framebuffer *fb)
> +{
> +	struct intel_crtc_config *pipe_config;
> +	unsigned modeset_pipes, prepare_pipes, disable_pipes;
> +
> +	pipe_config = intel_modeset_compute_config(crtc, mode, fb,
> +						   &modeset_pipes,
> +						   &prepare_pipes,
> +						   &disable_pipes);
> +
> +	if (IS_ERR(pipe_config))
> +		return PTR_ERR(pipe_config);
> +
> +	return intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
> +				    modeset_pipes, prepare_pipes,
> +				    disable_pipes);
> +}

intel_set_mode() -> intel_set_mode_pipes() -> __intel_set_mode() wins
this morning's prize for causing confusion.

Does it make sense to wrap intel_crtc_config + pipes in a new struct to
avoid passing 4 new parameters down the chain? Or will that just be
extra churn to be rewritten by atomic?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode v3

[Daniel Vetter]

On Thu, Nov 06, 2014 at 09:04:14AM +0000, Chris Wilson wrote:
> On Wed, Nov 05, 2014 at 02:26:06PM -0800, Jesse Barnes wrote:
> > +static int intel_set_mode(struct drm_crtc *crtc,
> > +			  struct drm_display_mode *mode,
> > +			  int x, int y, struct drm_framebuffer *fb)
> > +{
> > +	struct intel_crtc_config *pipe_config;
> > +	unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > +
> > +	pipe_config = intel_modeset_compute_config(crtc, mode, fb,
> > +						   &modeset_pipes,
> > +						   &prepare_pipes,
> > +						   &disable_pipes);
> > +
> > +	if (IS_ERR(pipe_config))
> > +		return PTR_ERR(pipe_config);
> > +
> > +	return intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
> > +				    modeset_pipes, prepare_pipes,
> > +				    disable_pipes);
> > +}
> 
> intel_set_mode() -> intel_set_mode_pipes() -> __intel_set_mode() wins
> this morning's prize for causing confusion.
> 
> Does it make sense to wrap intel_crtc_config + pipes in a new struct to
> avoid passing 4 new parameters down the chain? Or will that just be
> extra churn to be rewritten by atomic?

Atomic has one big structure to track all the updated per-object states
(for crtcs, planes & connectors). Atm there's no provisions for
subclassing it, but would be trivial to add. Then we could throw all this
additional i915 state in there. I guess we might as well start with that.

One funny problem btw is all these ->new_ pointers we sprinkle all over
the place. Upstream atomic has completely free-standing new state objects,
with a bunch of helpers to fetch the new state for a given object from the
overall atomic update structure. So we have a bit of an impendance
mismatch. But I think we just need to handle that by setting (and
resetting when we don't actually commit the new state) these pointers when
entering the i915 atomic backend.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm/i915: use compute_config in set_config v3

[Ander Conselvan de Oliveira]

On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> This will allow us to consult more info before deciding whether to flip
> or do a full mode set.
>
> v2:
>    - don't use uninitialized or incorrect pipe masks in set_config
>      failure path (Ander)
> v3:
>    - fixup for pipe_config changes in compute_config (Jesse)
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++++---
>   1 file changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a3ebab8..cb96f11 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>   	struct drm_device *dev;
>   	struct drm_mode_set save_set;
>   	struct intel_set_config *config;
> +	struct intel_crtc_config *pipe_config;
> +	unsigned modeset_pipes, prepare_pipes, disable_pipes;
>   	int ret;
>
>   	BUG_ON(!set);
> @@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>   	if (ret)
>   		goto fail;
>
> +	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
> +						   set->fb,
> +						   &modeset_pipes,
> +						   &prepare_pipes,
> +						   &disable_pipes);
> +	if (IS_ERR(pipe_config))
> +		goto fail;
> +
> +	/* set_mode will free it in the mode_changed case */
> +	if (!config->mode_changed)
> +		kfree(pipe_config);
> +
>   	if (config->mode_changed) {
> -		ret = intel_set_mode(set->crtc, set->mode,
> -				     set->x, set->y, set->fb);
> +		ret = intel_set_mode_pipes(set->crtc, set->mode,
> +					   set->x, set->y, set->fb, pipe_config,
> +					   modeset_pipes, prepare_pipes,
> +					   disable_pipes);
>   	} else if (config->fb_changed) {
>   		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
>
> @@ -13198,6 +13214,20 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>   		for_each_pipe(dev_priv, pipe) {
>   			struct drm_crtc *crtc =
>   				dev_priv->pipe_to_crtc_mapping[pipe];
> +			struct intel_crtc_config *pipe_config;
> +			unsigned modeset_pipes, prepare_pipes, disable_pipes;
> +
> +			pipe_config = intel_modeset_compute_config(crtc,
> +								   &crtc->mode,
> +								   crtc->primary->fb,
> +								   &modeset_pipes,
> +								   &prepare_pipes,
> +								   &disable_pipes);
> +			if (IS_ERR(pipe_config)) {
> +				DRM_DEBUG_KMS("prepare failed\n");
> +				kfree(pipe_config);
> +				goto out;
> +			}
>
>   			intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
>   				       crtc->primary->fb);

Did you mean to convert this to intel_set_mode_pipes()? Since you 
changed intel_set_mode() to do exactly the same thing as above, the 
whole hunk seems unnecessary unless you really care about logging that 
it was the prepare step that failed.

Ander

> @@ -13205,7 +13235,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>   	} else {
>   		intel_modeset_update_staged_output_state(dev);
>   	}
> -
> +out:
>   	intel_modeset_check_state(dev);
>   }
>
>

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

Re: [PATCH 2/6] drm/i915: use compute_config in set_config v3

[Jesse Barnes]

On Fri, 07 Nov 2014 11:41:48 +0200
Ander Conselvan de Oliveira <conselvan2@gmail.com> wrote:

> On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> > This will allow us to consult more info before deciding whether to flip
> > or do a full mode set.
> >
> > v2:
> >    - don't use uninitialized or incorrect pipe masks in set_config
> >      failure path (Ander)
> > v3:
> >    - fixup for pipe_config changes in compute_config (Jesse)
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++++---
> >   1 file changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index a3ebab8..cb96f11 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> >   	struct drm_device *dev;
> >   	struct drm_mode_set save_set;
> >   	struct intel_set_config *config;
> > +	struct intel_crtc_config *pipe_config;
> > +	unsigned modeset_pipes, prepare_pipes, disable_pipes;
> >   	int ret;
> >
> >   	BUG_ON(!set);
> > @@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> >   	if (ret)
> >   		goto fail;
> >
> > +	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
> > +						   set->fb,
> > +						   &modeset_pipes,
> > +						   &prepare_pipes,
> > +						   &disable_pipes);
> > +	if (IS_ERR(pipe_config))
> > +		goto fail;
> > +
> > +	/* set_mode will free it in the mode_changed case */
> > +	if (!config->mode_changed)
> > +		kfree(pipe_config);
> > +
> >   	if (config->mode_changed) {
> > -		ret = intel_set_mode(set->crtc, set->mode,
> > -				     set->x, set->y, set->fb);
> > +		ret = intel_set_mode_pipes(set->crtc, set->mode,
> > +					   set->x, set->y, set->fb, pipe_config,
> > +					   modeset_pipes, prepare_pipes,
> > +					   disable_pipes);
> >   	} else if (config->fb_changed) {
> >   		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
> >
> > @@ -13198,6 +13214,20 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> >   		for_each_pipe(dev_priv, pipe) {
> >   			struct drm_crtc *crtc =
> >   				dev_priv->pipe_to_crtc_mapping[pipe];
> > +			struct intel_crtc_config *pipe_config;
> > +			unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > +
> > +			pipe_config = intel_modeset_compute_config(crtc,
> > +								   &crtc->mode,
> > +								   crtc->primary->fb,
> > +								   &modeset_pipes,
> > +								   &prepare_pipes,
> > +								   &disable_pipes);
> > +			if (IS_ERR(pipe_config)) {
> > +				DRM_DEBUG_KMS("prepare failed\n");
> > +				kfree(pipe_config);
> > +				goto out;
> > +			}
> >
> >   			intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
> >   				       crtc->primary->fb);
> 
> Did you mean to convert this to intel_set_mode_pipes()? Since you 
> changed intel_set_mode() to do exactly the same thing as above, the 
> whole hunk seems unnecessary unless you really care about logging that 
> it was the prepare step that failed.

Ah yeah forgot to drop the above bits.  I think it's easier to just
call set_mode here rather than duplicating more compute_config bits.
I'll do that.

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

[PATCH] drm/i915: use compute_config in set_config v4

[Jesse Barnes]

This will allow us to consult more info before deciding whether to flip
or do a full mode set.

v2:
  - don't use uninitialized or incorrect pipe masks in set_config
    failure path (Ander)
v3:
  - fixup for pipe_config changes in compute_config (Jesse)
v4:
  - drop spurious hunk in force restore path (Ander)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a3ebab8..72123e0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	struct drm_device *dev;
 	struct drm_mode_set save_set;
 	struct intel_set_config *config;
+	struct intel_crtc_config *pipe_config;
+	unsigned modeset_pipes, prepare_pipes, disable_pipes;
 	int ret;
 
 	BUG_ON(!set);
@@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (ret)
 		goto fail;
 
+	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
+						   set->fb,
+						   &modeset_pipes,
+						   &prepare_pipes,
+						   &disable_pipes);
+	if (IS_ERR(pipe_config))
+		goto fail;
+
+	/* set_mode will free it in the mode_changed case */
+	if (!config->mode_changed)
+		kfree(pipe_config);
+
 	if (config->mode_changed) {
-		ret = intel_set_mode(set->crtc, set->mode,
-				     set->x, set->y, set->fb);
+		ret = intel_set_mode_pipes(set->crtc, set->mode,
+					   set->x, set->y, set->fb, pipe_config,
+					   modeset_pipes, prepare_pipes,
+					   disable_pipes);
 	} else if (config->fb_changed) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
-- 
1.9.1

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

Re: [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode v3

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> This allows us to calculate the full pipe config before we do any mode
> setting work.
> 
> v2:
>    - clarify comments about global vs. per-crtc mode set (Ander)
>    - clean up unnecessary pipe_config = NULL setting (Ander)
> v3:
>    - fix pipe_config handling (alloc in compute_config, free in set_mode) (Jesse)
>    - fix arg order in set_mode (Jesse)
>    - fix failure path of set_config (Ander)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 105 ++++++++++++++++++++++++-----------
>   1 file changed, 74 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a9df85f..a3ebab8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10656,45 +10656,60 @@ static void update_scanline_offset(struct intel_crtc *crtc)
>   		crtc->scanline_offset = 1;
>   }
>   
> +static struct intel_crtc_config *
> +intel_modeset_compute_config(struct drm_crtc *crtc,
> +			     struct drm_display_mode *mode,
> +			     struct drm_framebuffer *fb,
> +			     unsigned *modeset_pipes,
> +			     unsigned *prepare_pipes,
> +			     unsigned *disable_pipes)
> +{
> +	struct intel_crtc_config *pipe_config = NULL;
> +
> +	intel_modeset_affected_pipes(crtc, modeset_pipes,
> +				     prepare_pipes, disable_pipes);
> +
> +	if ((*modeset_pipes) == 0)
> +		goto out;
> +
> +	/*
> +	 * Note this needs changes when we start tracking multiple modes
> +	 * and crtcs.  At that point we'll need to compute the whole config
> +	 * (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;
> +	}
> +	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> +			       "[modeset]");
> +	to_intel_crtc(crtc)->new_config = pipe_config;
> +
> +out:
> +	return pipe_config;
> +}
> +
>   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 intel_crtc_config *pipe_config,
> +			    unsigned modeset_pipes,
> +			    unsigned prepare_pipes,
> +			    unsigned disable_pipes)
>   {
>   	struct drm_device *dev = crtc->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct drm_display_mode *saved_mode;
> -	struct intel_crtc_config *pipe_config = NULL;
>   	struct intel_crtc *intel_crtc;
> -	unsigned disable_pipes, prepare_pipes, modeset_pipes;
>   	int ret = 0;
>   
>   	saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
>   	if (!saved_mode)
>   		return -ENOMEM;
>   
> -	intel_modeset_affected_pipes(crtc, &modeset_pipes,
> -				     &prepare_pipes, &disable_pipes);
> -
>   	*saved_mode = crtc->mode;
>   
> -	/* Hack: Because we don't (yet) support global modeset on multiple
> -	 * crtcs, we don't keep track of the new mode for more than one crtc.
> -	 * Hence simply check whether any bit is set in modeset_pipes in all the
> -	 * pieces of code that are not yet converted to deal with mutliple crtcs
> -	 * changing their mode at the same time. */
> -	if (modeset_pipes) {
> -		pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> -		if (IS_ERR(pipe_config)) {
> -			ret = PTR_ERR(pipe_config);
> -			pipe_config = NULL;
> -
> -			goto out;
> -		}
> -		intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> -				       "[modeset]");
> -		to_intel_crtc(crtc)->new_config = pipe_config;
> -	}
> -
>   	/*
>   	 * See if the config requires any additional preparation, e.g.
>   	 * to adjust global state with pipes off.  We need to do this
> @@ -10719,6 +10734,10 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>   
>   	/* crtc->mode is already used by the ->mode_set callbacks, hence we need
>   	 * to set it here already despite that we pass it down the callchain.
> +	 *
> +	 * Note we'll need to fix this up when we start tracking multiple
> +	 * pipes; here we assume a single modeset_pipe and only track the
> +	 * single crtc and mode.
>   	 */
>   	if (modeset_pipes) {
>   		crtc->mode = *mode;
> @@ -10787,19 +10806,23 @@ done:
>   	if (ret && crtc->enabled)
>   		crtc->mode = *saved_mode;
>   
> -out:
>   	kfree(pipe_config);
>   	kfree(saved_mode);
>   	return ret;
>   }
>   
> -static int intel_set_mode(struct drm_crtc *crtc,
> -			  struct drm_display_mode *mode,
> -			  int x, int y, struct drm_framebuffer *fb)
> +static int intel_set_mode_pipes(struct drm_crtc *crtc,
> +				struct drm_display_mode *mode,
> +				int x, int y, struct drm_framebuffer *fb,
> +				struct intel_crtc_config *pipe_config,
> +				unsigned modeset_pipes,
> +				unsigned prepare_pipes,
> +				unsigned disable_pipes)
>   {
>   	int ret;
>   
> -	ret = __intel_set_mode(crtc, mode, x, y, fb);
> +	ret = __intel_set_mode(crtc, mode, x, y, fb, pipe_config, modeset_pipes,
> +			       prepare_pipes, disable_pipes);
>   
>   	if (ret == 0)
>   		intel_modeset_check_state(crtc->dev);
> @@ -10807,6 +10830,26 @@ static int intel_set_mode(struct drm_crtc *crtc,
>   	return ret;
>   }
>   
> +static int intel_set_mode(struct drm_crtc *crtc,
> +			  struct drm_display_mode *mode,
> +			  int x, int y, struct drm_framebuffer *fb)
> +{
> +	struct intel_crtc_config *pipe_config;
> +	unsigned modeset_pipes, prepare_pipes, disable_pipes;
> +
> +	pipe_config = intel_modeset_compute_config(crtc, mode, fb,
> +						   &modeset_pipes,
> +						   &prepare_pipes,
> +						   &disable_pipes);
> +
> +	if (IS_ERR(pipe_config))
> +		return PTR_ERR(pipe_config);
> +
> +	return intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
> +				    modeset_pipes, prepare_pipes,
> +				    disable_pipes);
> +}
> +
>   void intel_crtc_restore_mode(struct drm_crtc *crtc)
>   {
>   	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb);
> @@ -13156,8 +13199,8 @@ 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_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
> +				       crtc->primary->fb);
>   		}
>   	} else {
>   		intel_modeset_update_staged_output_state(dev);
> 

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

Re: [PATCH] drm/i915: use compute_config in set_config v4

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

On 11/07/2014 11:11 PM, Jesse Barnes wrote:
> This will allow us to consult more info before deciding whether to flip
> or do a full mode set.
> 
> v2:
>    - don't use uninitialized or incorrect pipe masks in set_config
>      failure path (Ander)
> v3:
>    - fixup for pipe_config changes in compute_config (Jesse)
> v4:
>    - drop spurious hunk in force restore path (Ander)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a3ebab8..72123e0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>   	struct drm_device *dev;
>   	struct drm_mode_set save_set;
>   	struct intel_set_config *config;
> +	struct intel_crtc_config *pipe_config;
> +	unsigned modeset_pipes, prepare_pipes, disable_pipes;
>   	int ret;
>   
>   	BUG_ON(!set);
> @@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>   	if (ret)
>   		goto fail;
>   
> +	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
> +						   set->fb,
> +						   &modeset_pipes,
> +						   &prepare_pipes,
> +						   &disable_pipes);
> +	if (IS_ERR(pipe_config))
> +		goto fail;
> +
> +	/* set_mode will free it in the mode_changed case */
> +	if (!config->mode_changed)
> +		kfree(pipe_config);
> +
>   	if (config->mode_changed) {
> -		ret = intel_set_mode(set->crtc, set->mode,
> -				     set->x, set->y, set->fb);
> +		ret = intel_set_mode_pipes(set->crtc, set->mode,
> +					   set->x, set->y, set->fb, pipe_config,
> +					   modeset_pipes, prepare_pipes,
> +					   disable_pipes);
>   	} else if (config->fb_changed) {
>   		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
>   
> 

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

Re: [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> This is useful for checking things later.
> 
> v2:
>    - fix hsw infoframe enabled check (Ander)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_drv.h  |  4 +++
>   drivers/gpu/drm/i915/intel_hdmi.c | 62 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d53ac23..8aa80e1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -292,6 +292,9 @@ struct intel_crtc_config {
>   	 * between pch encoders and cpu encoders. */
>   	bool has_pch_encoder;
>   
> +	/* Are we sending infoframes on the attached port */
> +	bool has_infoframe;
> +
>   	/* CPU Transcoder for the pipe. Currently this can only differ from the
>   	 * pipe on Haswell (where we have a special eDP transcoder). */
>   	enum transcoder cpu_transcoder;
> @@ -543,6 +546,7 @@ struct intel_hdmi {
>   	void (*set_infoframes)(struct drm_encoder *encoder,
>   			       bool enable,
>   			       struct drm_display_mode *adjusted_mode);
> +	bool (*infoframe_enabled)(struct drm_encoder *encoder);
>   };
>   
>   struct intel_dp_mst_encoder;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 07b5ebd..994237a 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -166,6 +166,15 @@ static void g4x_write_infoframe(struct drm_encoder *encoder,
>   	POSTING_READ(VIDEO_DIP_CTL);
>   }
>   
> +static bool g4x_infoframe_enabled(struct drm_encoder *encoder)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 val = I915_READ(VIDEO_DIP_CTL);
> +
> +	return val & VIDEO_DIP_ENABLE;
> +}
> +
>   static void ibx_write_infoframe(struct drm_encoder *encoder,
>   				enum hdmi_infoframe_type type,
>   				const void *frame, ssize_t len)
> @@ -204,6 +213,17 @@ static void ibx_write_infoframe(struct drm_encoder *encoder,
>   	POSTING_READ(reg);
>   }
>   
> +static bool ibx_infoframe_enabled(struct drm_encoder *encoder)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> +	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
> +	u32 val = I915_READ(reg);
> +
> +	return val & VIDEO_DIP_ENABLE;
> +}
> +
>   static void cpt_write_infoframe(struct drm_encoder *encoder,
>   				enum hdmi_infoframe_type type,
>   				const void *frame, ssize_t len)
> @@ -245,6 +265,17 @@ static void cpt_write_infoframe(struct drm_encoder *encoder,
>   	POSTING_READ(reg);
>   }
>   
> +static bool cpt_infoframe_enabled(struct drm_encoder *encoder)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> +	int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
> +	u32 val = I915_READ(reg);
> +
> +	return val & VIDEO_DIP_ENABLE;
> +}
> +
>   static void vlv_write_infoframe(struct drm_encoder *encoder,
>   				enum hdmi_infoframe_type type,
>   				const void *frame, ssize_t len)
> @@ -283,6 +314,17 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
>   	POSTING_READ(reg);
>   }
>   
> +static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> +	int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
> +	u32 val = I915_READ(reg);
> +
> +	return val & VIDEO_DIP_ENABLE;
> +}
> +
>   static void hsw_write_infoframe(struct drm_encoder *encoder,
>   				enum hdmi_infoframe_type type,
>   				const void *frame, ssize_t len)
> @@ -320,6 +362,18 @@ static void hsw_write_infoframe(struct drm_encoder *encoder,
>   	POSTING_READ(ctl_reg);
>   }
>   
> +static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
> +{
> +	struct drm_device *dev = encoder->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
> +	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
> +	u32 val = I915_READ(ctl_reg);
> +
> +	return val & (VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
> +		      VIDEO_DIP_ENABLE_VS_HSW);
> +}
> +
>   /*
>    * The data we write to the DIP data buffer registers is 1 byte bigger than the
>    * HDMI infoframe size because of an ECC/reserved byte at position 3 (starting
> @@ -732,6 +786,9 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>   	if (tmp & HDMI_MODE_SELECT_HDMI)
>   		pipe_config->has_hdmi_sink = true;
>   
> +	if (intel_hdmi->infoframe_enabled(&encoder->base))
> +		pipe_config->has_infoframe = true;
> +
>   	if (tmp & SDVO_AUDIO_ENABLE)
>   		pipe_config->has_audio = true;
>   
> @@ -1616,18 +1673,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>   	if (IS_VALLEYVIEW(dev)) {
>   		intel_hdmi->write_infoframe = vlv_write_infoframe;
>   		intel_hdmi->set_infoframes = vlv_set_infoframes;
> +		intel_hdmi->infoframe_enabled = vlv_infoframe_enabled;
>   	} else if (IS_G4X(dev)) {
>   		intel_hdmi->write_infoframe = g4x_write_infoframe;
>   		intel_hdmi->set_infoframes = g4x_set_infoframes;
> +		intel_hdmi->infoframe_enabled = g4x_infoframe_enabled;
>   	} else if (HAS_DDI(dev)) {
>   		intel_hdmi->write_infoframe = hsw_write_infoframe;
>   		intel_hdmi->set_infoframes = hsw_set_infoframes;
> +		intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
>   	} else if (HAS_PCH_IBX(dev)) {
>   		intel_hdmi->write_infoframe = ibx_write_infoframe;
>   		intel_hdmi->set_infoframes = ibx_set_infoframes;
> +		intel_hdmi->infoframe_enabled = ibx_infoframe_enabled;
>   	} else {
>   		intel_hdmi->write_infoframe = cpt_write_infoframe;
>   		intel_hdmi->set_infoframes = cpt_set_infoframes;
> +		intel_hdmi->infoframe_enabled = cpt_infoframe_enabled;
>   	}
>   
>   	if (HAS_DDI(dev))
> 

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

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> If these change (e.g. after a modeset following a fastboot), we need to
> do a full mode set.
> 
> v2:
>    - put under pipe_config check so we don't deref a null state (Jesse)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cb96f11..c86eee6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>   						   &modeset_pipes,
>   						   &prepare_pipes,
>   						   &disable_pipes);
> -	if (IS_ERR(pipe_config))
> +	if (IS_ERR(pipe_config)) {
>   		goto fail;
> +	} else if (pipe_config) {
> +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> +		    to_intel_crtc(set->crtc)->config.has_audio)
> +			config->mode_changed = true;
> +
> +		/* Force mode sets for any infoframe stuff */
> +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> +			config->mode_changed = true;
> +	}
>   
>   	/* set_mode will free it in the mode_changed case */
>   	if (!config->mode_changed)
> 

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

Re: [PATCH 5/6] drm/i915: update pipe size at set_config time

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> This only affects the fastboot path as-is.  In that case, we simply need
> to make sure that we update the pipe size at the first mode set.  Rather
> than putting it off until we decide to flip (if indeed we do end up
> flipping), update the pipe size as appropriate a bit earlier in the
> set_config call.
> 
> This sets us up for better pipe tracking in later patches.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c86eee6..3f1515d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2906,8 +2906,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
>   		return ret;
>   	}
>   
> -	intel_update_pipe_size(intel_crtc);
> -
>   	dev_priv->display.update_primary_plane(crtc, fb, x, y);
>   
>   	if (intel_crtc->active)
> @@ -11247,6 +11245,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>   	if (!config->mode_changed)
>   		kfree(pipe_config);
>   
> +	intel_update_pipe_size(to_intel_crtc(set->crtc));
> +
>   	if (config->mode_changed) {
>   		ret = intel_set_mode_pipes(set->crtc, set->mode,
>   					   set->x, set->y, set->fb, pipe_config,
> 

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

Re: [PATCH 6/6] drm/i915: calculate pfit changes in set_config v2

[Ander Conselvan de Oliveira]

On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> This should allow us to avoid mode sets for some panel fitter config
> changes.
>
> v2:
>    - fixup pfit comment (Ander)
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++++++++++-------
>   1 file changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3f1515d..49281d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
>   		return;
>
>   	/*
> -	 * Update pipe size and adjust fitter if needed: the reason for this is
> -	 * that in compute_mode_changes we check the native mode (not the pfit
> -	 * mode) to see if we can flip rather than do a full mode set. In the
> -	 * fastboot case, we'll flip, but if we don't update the pipesrc and
> -	 * pfit state, we'll end up with a big fb scanned out into the wrong
> -	 * sized surface.
> -	 *
> -	 * To fix this properly, we need to hoist the checks up into
> -	 * compute_mode_changes (or above), check the actual pfit state and
> -	 * whether the platform allows pfit disable with pipe active, and only
> -	 * then update the pipesrc and pfit state, even on the flip path.
> +	 * See intel_pfit_changed for info on when we're allowed to
> +	 * do this w/o a pipe shutdown.
>   	 */
>
>   	adjusted_mode = &crtc->config.adjusted_mode;
> @@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
>   	crtc->new_config = NULL;
>   }
>
> +/* Do we need a mode set due to pfit changes? */
> +static bool intel_pfit_changed(struct drm_device *dev,
> +			       struct intel_crtc_config *new_config,
> +			       struct intel_crtc_config *cur_config)
> +{
> +	bool ret = false;
> +
> +	if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
> +		/*
> +		 * On PCH platforms we can disable pfit w/o a pipe shutdown,
> +		 * otherwise we'll need a mode set.
> +		 */
> +		if (new_config->pch_pfit.enabled &&
> +		    cur_config->pch_pfit.enabled)
> +			ret = false;
> +		else if (new_config->pch_pfit.enabled &&
> +			 !cur_config->pch_pfit.enabled)
> +			ret = true;
> +		else if (!new_config->pch_pfit.enabled &&
> +			 cur_config->pch_pfit.enabled)
> +			ret = false;
> +		else if (!new_config->pch_pfit.enabled &&
> +			 !cur_config->pch_pfit.enabled)
> +			ret = false;
> +	} else {
> +		bool new_enabled, old_enabled;
> +
> +		new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
> +		old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
> +
> +		/* 9xx only needs a shutdown to disable pfit */
> +		if (new_enabled && old_enabled)
> +			ret = false;
> +		else if (new_enabled && !old_enabled)
> +			ret = false;
> +		else if (!new_enabled && old_enabled)
> +			ret = true;
> +		else if (!new_enabled && !old_enabled)
> +			ret = false;
> +	}

Maybe I missed something, but I couldn't find anything in the 
documentation the supports the claim above.  However, [1] and [2] read 
that "[p]anel fitting should be enabled or disabled before the pipe is 
enabled" on the documentation for the PFIT_CONTROL.


[1] 
https://01.org/linuxgraphics/sites/default/files/documentation/g45_vol_3_register_0_0.pdf
[2] 
https://01.org/linuxgraphics/sites/default/files/documentation/965_g35_vol_3_display_registers_updated_1.pdf


Ander

> +
> +	return ret;
> +}
> +
>   static int intel_crtc_set_config(struct drm_mode_set *set)
>   {
>   	struct drm_device *dev;
> @@ -11239,6 +11274,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>   		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
>   		    to_intel_crtc(set->crtc)->config.has_infoframe)
>   			config->mode_changed = true;
> +
> +		if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config,
> +				       &to_intel_crtc(set->crtc)->config))
> +			config->mode_changed = true;
>   	}
>
>   	/* set_mode will free it in the mode_changed case */
>

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

Re: [PATCH 6/6] drm/i915: calculate pfit changes in set_config v2

[Jesse Barnes]

On Mon, 10 Nov 2014 18:20:56 +0200
Ander Conselvan de Oliveira <conselvan2@gmail.com> wrote:

> On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> > This should allow us to avoid mode sets for some panel fitter config
> > changes.
> >
> > v2:
> >    - fixup pfit comment (Ander)
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 61 +++++++++++++++++++++++++++++-------
> >   1 file changed, 50 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3f1515d..49281d7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
> >   		return;
> >
> >   	/*
> > -	 * Update pipe size and adjust fitter if needed: the reason for this is
> > -	 * that in compute_mode_changes we check the native mode (not the pfit
> > -	 * mode) to see if we can flip rather than do a full mode set. In the
> > -	 * fastboot case, we'll flip, but if we don't update the pipesrc and
> > -	 * pfit state, we'll end up with a big fb scanned out into the wrong
> > -	 * sized surface.
> > -	 *
> > -	 * To fix this properly, we need to hoist the checks up into
> > -	 * compute_mode_changes (or above), check the actual pfit state and
> > -	 * whether the platform allows pfit disable with pipe active, and only
> > -	 * then update the pipesrc and pfit state, even on the flip path.
> > +	 * See intel_pfit_changed for info on when we're allowed to
> > +	 * do this w/o a pipe shutdown.
> >   	 */
> >
> >   	adjusted_mode = &crtc->config.adjusted_mode;
> > @@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
> >   	crtc->new_config = NULL;
> >   }
> >
> > +/* Do we need a mode set due to pfit changes? */
> > +static bool intel_pfit_changed(struct drm_device *dev,
> > +			       struct intel_crtc_config *new_config,
> > +			       struct intel_crtc_config *cur_config)
> > +{
> > +	bool ret = false;
> > +
> > +	if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
> > +		/*
> > +		 * On PCH platforms we can disable pfit w/o a pipe shutdown,
> > +		 * otherwise we'll need a mode set.
> > +		 */
> > +		if (new_config->pch_pfit.enabled &&
> > +		    cur_config->pch_pfit.enabled)
> > +			ret = false;
> > +		else if (new_config->pch_pfit.enabled &&
> > +			 !cur_config->pch_pfit.enabled)
> > +			ret = true;
> > +		else if (!new_config->pch_pfit.enabled &&
> > +			 cur_config->pch_pfit.enabled)
> > +			ret = false;
> > +		else if (!new_config->pch_pfit.enabled &&
> > +			 !cur_config->pch_pfit.enabled)
> > +			ret = false;
> > +	} else {
> > +		bool new_enabled, old_enabled;
> > +
> > +		new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
> > +		old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
> > +
> > +		/* 9xx only needs a shutdown to disable pfit */
> > +		if (new_enabled && old_enabled)
> > +			ret = false;
> > +		else if (new_enabled && !old_enabled)
> > +			ret = false;
> > +		else if (!new_enabled && old_enabled)
> > +			ret = true;
> > +		else if (!new_enabled && !old_enabled)
> > +			ret = false;
> > +	}
> 
> Maybe I missed something, but I couldn't find anything in the 
> documentation the supports the claim above.  However, [1] and [2] read 
> that "[p]anel fitting should be enabled or disabled before the pipe is 
> enabled" on the documentation for the PFIT_CONTROL.
> 
> 
> [1] 
> https://01.org/linuxgraphics/sites/default/files/documentation/g45_vol_3_register_0_0.pdf
> [2] 
> https://01.org/linuxgraphics/sites/default/files/documentation/965_g35_vol_3_display_registers_updated_1.pdf

Yeah the docs are extra conservative on this.  But from memory, this is
what 965 allowed.  It would be good to do some extra testing though;
915/945 may allow both pfit enable and disable without a pipe shutdown.

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

Re: [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Daniel Vetter]

On Wed, Nov 05, 2014 at 02:26:08PM -0800, Jesse Barnes wrote:
> This is useful for checking things later.
> 
> v2:
>   - fix hsw infoframe enabled check (Ander)
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  4 +++
>  drivers/gpu/drm/i915/intel_hdmi.c | 62 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d53ac23..8aa80e1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -292,6 +292,9 @@ struct intel_crtc_config {
>  	 * between pch encoders and cpu encoders. */
>  	bool has_pch_encoder;
>  
> +	/* Are we sending infoframes on the attached port */
> +	bool has_infoframe;

The cross-checking of this new hw state is missing. I've added that while
applying.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: update pipe size at set_config time

[Daniel Vetter]

On Mon, Nov 10, 2014 at 06:09:45PM +0200, Ander Conselvan de Oliveira wrote:
> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> 
> On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> > This only affects the fastboot path as-is.  In that case, we simply need
> > to make sure that we update the pipe size at the first mode set.  Rather
> > than putting it off until we decide to flip (if indeed we do end up
> > flipping), update the pipe size as appropriate a bit earlier in the
> > set_config call.
> > 
> > This sets us up for better pipe tracking in later patches.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Merged up to this one here, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Ville Syrjälä]

On Tue, Nov 11, 2014 at 04:00:12PM +0100, Daniel Vetter wrote:
> On Wed, Nov 05, 2014 at 02:26:08PM -0800, Jesse Barnes wrote:
> > This is useful for checking things later.
> > 
> > v2:
> >   - fix hsw infoframe enabled check (Ander)
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h  |  4 +++
> >  drivers/gpu/drm/i915/intel_hdmi.c | 62 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index d53ac23..8aa80e1 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -292,6 +292,9 @@ struct intel_crtc_config {
> >  	 * between pch encoders and cpu encoders. */
> >  	bool has_pch_encoder;
> >  
> > +	/* Are we sending infoframes on the attached port */
> > +	bool has_infoframe;
> 
> The cross-checking of this new hw state is missing. I've added that while
> applying.

Where is the compute_config part of this? I can't see it in any patch,
but maybe I'm just blind.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Daniel Vetter]

On Tue, Nov 11, 2014 at 05:19:46PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 11, 2014 at 04:00:12PM +0100, Daniel Vetter wrote:
> > On Wed, Nov 05, 2014 at 02:26:08PM -0800, Jesse Barnes wrote:
> > > This is useful for checking things later.
> > > 
> > > v2:
> > >   - fix hsw infoframe enabled check (Ander)
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/i915/intel_drv.h  |  4 +++
> > >  drivers/gpu/drm/i915/intel_hdmi.c | 62 +++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 66 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index d53ac23..8aa80e1 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -292,6 +292,9 @@ struct intel_crtc_config {
> > >  	 * between pch encoders and cpu encoders. */
> > >  	bool has_pch_encoder;
> > >  
> > > +	/* Are we sending infoframes on the attached port */
> > > +	bool has_infoframe;
> > 
> > The cross-checking of this new hw state is missing. I've added that while
> > applying.
> 
> Where is the compute_config part of this? I can't see it in any patch,
> but maybe I'm just blind.

Yeah, seems to be missing, thanks for catching this. Jesse/Ander, can you
pls supply a fixup quickly - I'd need to drop the series until this one
otherwise I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Jesse Barnes]

On Tue, 11 Nov 2014 16:23:03 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Nov 11, 2014 at 05:19:46PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 11, 2014 at 04:00:12PM +0100, Daniel Vetter wrote:
> > > On Wed, Nov 05, 2014 at 02:26:08PM -0800, Jesse Barnes wrote:
> > > > This is useful for checking things later.
> > > > 
> > > > v2:
> > > >   - fix hsw infoframe enabled check (Ander)
> > > > 
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_drv.h  |  4 +++
> > > >  drivers/gpu/drm/i915/intel_hdmi.c | 62 +++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 66 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > > index d53ac23..8aa80e1 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -292,6 +292,9 @@ struct intel_crtc_config {
> > > >  	 * between pch encoders and cpu encoders. */
> > > >  	bool has_pch_encoder;
> > > >  
> > > > +	/* Are we sending infoframes on the attached port */
> > > > +	bool has_infoframe;
> > > 
> > > The cross-checking of this new hw state is missing. I've added that while
> > > applying.
> > 
> > Where is the compute_config part of this? I can't see it in any patch,
> > but maybe I'm just blind.
> 
> Yeah, seems to be missing, thanks for catching this. Jesse/Ander, can you
> pls supply a fixup quickly - I'd need to drop the series until this one
> otherwise I think.

Ah right missed the other side.  Will post a patch today.

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

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jani Nikula]

On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> If these change (e.g. after a modeset following a fastboot), we need to
> do a full mode set.
>
> v2:
>   - put under pipe_config check so we don't deref a null state (Jesse)
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cb96f11..c86eee6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  						   &modeset_pipes,
>  						   &prepare_pipes,
>  						   &disable_pipes);
> -	if (IS_ERR(pipe_config))
> +	if (IS_ERR(pipe_config)) {
>  		goto fail;
> +	} else if (pipe_config) {
> +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> +		    to_intel_crtc(set->crtc)->config.has_audio)
> +			config->mode_changed = true;
> +
> +		/* Force mode sets for any infoframe stuff */
> +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> +		    to_intel_crtc(set->crtc)->config.has_infoframe)

Jesse, is "||" correct here? This is bound to force a lot of mode sets.

See https://bugs.freedesktop.org/show_bug.cgi?id=86683

BR,
Jani.


> +			config->mode_changed = true;
> +	}
>  
>  	/* set_mode will free it in the mode_changed case */
>  	if (!config->mode_changed)
> -- 
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jesse Barnes]

On Mon, 01 Dec 2014 12:25:45 +0200
Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > If these change (e.g. after a modeset following a fastboot), we need to
> > do a full mode set.
> >
> > v2:
> >   - put under pipe_config check so we don't deref a null state (Jesse)
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index cb96f11..c86eee6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> >  						   &modeset_pipes,
> >  						   &prepare_pipes,
> >  						   &disable_pipes);
> > -	if (IS_ERR(pipe_config))
> > +	if (IS_ERR(pipe_config)) {
> >  		goto fail;
> > +	} else if (pipe_config) {
> > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > +			config->mode_changed = true;
> > +
> > +		/* Force mode sets for any infoframe stuff */
> > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> 
> Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> 
> See https://bugs.freedesktop.org/show_bug.cgi?id=86683

Yeah, we talked about it on IRC.  I was being extra conservative here,
since we don't yet have code to analyze whether we need a full mode set
due to something that will require an infoframe change.  Currently, the
only way for us to write a new infoframe is through a mode set, and the
old code would just happen to do a mode set most of the time we wanted
it to (i.e. when the mode timings changed), but I don't think we'd ever
thought of the implications of infoframe changes for stuff that doesn't
require a mode set.

We could drop this hunk and continue to play it fast & loose, but what
we really need here is a fuller check about whether we need a mode set
due to requiring an infoframe change, which means pulling them apart a
bit.

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

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Chris Wilson]

On Mon, Dec 01, 2014 at 08:04:30AM -0800, Jesse Barnes wrote:
> On Mon, 01 Dec 2014 12:25:45 +0200
> Jani Nikula <jani.nikula@linux.intel.com> wrote:
> 
> > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > If these change (e.g. after a modeset following a fastboot), we need to
> > > do a full mode set.
> > >
> > > v2:
> > >   - put under pipe_config check so we don't deref a null state (Jesse)
> > >
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index cb96f11..c86eee6 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> > >  						   &modeset_pipes,
> > >  						   &prepare_pipes,
> > >  						   &disable_pipes);
> > > -	if (IS_ERR(pipe_config))
> > > +	if (IS_ERR(pipe_config)) {
> > >  		goto fail;
> > > +	} else if (pipe_config) {
> > > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > > +			config->mode_changed = true;
> > > +
> > > +		/* Force mode sets for any infoframe stuff */
> > > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> > 
> > Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> > 
> > See https://bugs.freedesktop.org/show_bug.cgi?id=86683
> 
> Yeah, we talked about it on IRC.  I was being extra conservative here,
> since we don't yet have code to analyze whether we need a full mode set
> due to something that will require an infoframe change.  Currently, the
> only way for us to write a new infoframe is through a mode set, and the
> old code would just happen to do a mode set most of the time we wanted
> it to (i.e. when the mode timings changed), but I don't think we'd ever
> thought of the implications of infoframe changes for stuff that doesn't
> require a mode set.
> 
> We could drop this hunk and continue to play it fast & loose, but what
> we really need here is a fuller check about whether we need a mode set
> due to requiring an infoframe change, which means pulling them apart a
> bit.

We need to be conservative. If we spot extraneous modesets in the wild,
we will be motivated to fix it. As a compromise, if you include a

DRM_DEBUG_KMS("infoframes detected - forcing full modeset\n")

that should be sufficient clue for us to pick up the thread again later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Daniel Vetter]

On Mon, Dec 01, 2014 at 12:25:45PM +0200, Jani Nikula wrote:
> On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > If these change (e.g. after a modeset following a fastboot), we need to
> > do a full mode set.
> >
> > v2:
> >   - put under pipe_config check so we don't deref a null state (Jesse)
> >
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index cb96f11..c86eee6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> >  						   &modeset_pipes,
> >  						   &prepare_pipes,
> >  						   &disable_pipes);
> > -	if (IS_ERR(pipe_config))
> > +	if (IS_ERR(pipe_config)) {
> >  		goto fail;
> > +	} else if (pipe_config) {
> > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > +			config->mode_changed = true;
> > +
> > +		/* Force mode sets for any infoframe stuff */
> > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> 
> Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> 
> See https://bugs.freedesktop.org/show_bug.cgi?id=86683

Indeed. And I think the approach is the wrong way round. Imo we should
check whether the computed pipe_config matches and have a few specific
cases that we can handle when it mismatches (e.g. pipe_src_w/h). Plus some
special logic if quirk flags are set (so that we force a full modeset in
cases like this to get rid of the boot-up takever state quirk flag).

I think best course of action here is to revert this patch, also because
it's kinda at the 1w deadline with set for regression. Even though this
here is imo QA's fault for taking way too long to deliver the bisect
result and noticing the issue (the patch is almost 1 month old by now).

Jesse?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Daniel Vetter]

On Mon, Dec 01, 2014 at 04:09:07PM +0000, Chris Wilson wrote:
> On Mon, Dec 01, 2014 at 08:04:30AM -0800, Jesse Barnes wrote:
> > On Mon, 01 Dec 2014 12:25:45 +0200
> > Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > 
> > > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > > If these change (e.g. after a modeset following a fastboot), we need to
> > > > do a full mode set.
> > > >
> > > > v2:
> > > >   - put under pipe_config check so we don't deref a null state (Jesse)
> > > >
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> > > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > index cb96f11..c86eee6 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> > > >  						   &modeset_pipes,
> > > >  						   &prepare_pipes,
> > > >  						   &disable_pipes);
> > > > -	if (IS_ERR(pipe_config))
> > > > +	if (IS_ERR(pipe_config)) {
> > > >  		goto fail;
> > > > +	} else if (pipe_config) {
> > > > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > > > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > > > +			config->mode_changed = true;
> > > > +
> > > > +		/* Force mode sets for any infoframe stuff */
> > > > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > > > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> > > 
> > > Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> > > 
> > > See https://bugs.freedesktop.org/show_bug.cgi?id=86683
> > 
> > Yeah, we talked about it on IRC.  I was being extra conservative here,
> > since we don't yet have code to analyze whether we need a full mode set
> > due to something that will require an infoframe change.  Currently, the
> > only way for us to write a new infoframe is through a mode set, and the
> > old code would just happen to do a mode set most of the time we wanted
> > it to (i.e. when the mode timings changed), but I don't think we'd ever
> > thought of the implications of infoframe changes for stuff that doesn't
> > require a mode set.
> > 
> > We could drop this hunk and continue to play it fast & loose, but what
> > we really need here is a fuller check about whether we need a mode set
> > due to requiring an infoframe change, which means pulling them apart a
> > bit.
> 
> We need to be conservative. If we spot extraneous modesets in the wild,
> we will be motivated to fix it. As a compromise, if you include a
> 
> DRM_DEBUG_KMS("infoframes detected - forcing full modeset\n")
> 
> that should be sufficient clue for us to pick up the thread again later.

The problem is that with the current code we'll do a modeset for all hdmi
outputs, unconditionally. Even without fastboot=1 set. Which is too much
and a regression. E.g. just having virtual > screen and scrolling with the
mouse old-style would now result in a modeset each time we move one pixel.
Or at least that's my analysis here (didnt test).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Chris Wilson]

On Mon, Dec 01, 2014 at 05:30:41PM +0100, Daniel Vetter wrote:
> On Mon, Dec 01, 2014 at 04:09:07PM +0000, Chris Wilson wrote:
> > On Mon, Dec 01, 2014 at 08:04:30AM -0800, Jesse Barnes wrote:
> > > On Mon, 01 Dec 2014 12:25:45 +0200
> > > Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > > 
> > > > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > > > If these change (e.g. after a modeset following a fastboot), we need to
> > > > > do a full mode set.
> > > > >
> > > > > v2:
> > > > >   - put under pipe_config check so we don't deref a null state (Jesse)
> > > > >
> > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> > > > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > > index cb96f11..c86eee6 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> > > > >  						   &modeset_pipes,
> > > > >  						   &prepare_pipes,
> > > > >  						   &disable_pipes);
> > > > > -	if (IS_ERR(pipe_config))
> > > > > +	if (IS_ERR(pipe_config)) {
> > > > >  		goto fail;
> > > > > +	} else if (pipe_config) {
> > > > > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > > > > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > > > > +			config->mode_changed = true;
> > > > > +
> > > > > +		/* Force mode sets for any infoframe stuff */
> > > > > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > > > > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> > > > 
> > > > Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> > > > 
> > > > See https://bugs.freedesktop.org/show_bug.cgi?id=86683
> > > 
> > > Yeah, we talked about it on IRC.  I was being extra conservative here,
> > > since we don't yet have code to analyze whether we need a full mode set
> > > due to something that will require an infoframe change.  Currently, the
> > > only way for us to write a new infoframe is through a mode set, and the
> > > old code would just happen to do a mode set most of the time we wanted
> > > it to (i.e. when the mode timings changed), but I don't think we'd ever
> > > thought of the implications of infoframe changes for stuff that doesn't
> > > require a mode set.
> > > 
> > > We could drop this hunk and continue to play it fast & loose, but what
> > > we really need here is a fuller check about whether we need a mode set
> > > due to requiring an infoframe change, which means pulling them apart a
> > > bit.
> > 
> > We need to be conservative. If we spot extraneous modesets in the wild,
> > we will be motivated to fix it. As a compromise, if you include a
> > 
> > DRM_DEBUG_KMS("infoframes detected - forcing full modeset\n")
> > 
> > that should be sufficient clue for us to pick up the thread again later.
> 
> The problem is that with the current code we'll do a modeset for all hdmi
> outputs, unconditionally. Even without fastboot=1 set. Which is too much
> and a regression. E.g. just having virtual > screen and scrolling with the
> mouse old-style would now result in a modeset each time we move one pixel.
> Or at least that's my analysis here (didnt test).

Would it be just as easy to construct a scenario that had an infroframe
change that didn't get applied with the revert? Besides which a full
modeset on every pan should be good motiviation to make them nonblocking
(or at least use vblank workers to do the clenaup) ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jesse Barnes]

On Mon, 1 Dec 2014 17:16:25 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Dec 01, 2014 at 12:25:45PM +0200, Jani Nikula wrote:
> > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > If these change (e.g. after a modeset following a fastboot), we need to
> > > do a full mode set.
> > >
> > > v2:
> > >   - put under pipe_config check so we don't deref a null state (Jesse)
> > >
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index cb96f11..c86eee6 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> > >  						   &modeset_pipes,
> > >  						   &prepare_pipes,
> > >  						   &disable_pipes);
> > > -	if (IS_ERR(pipe_config))
> > > +	if (IS_ERR(pipe_config)) {
> > >  		goto fail;
> > > +	} else if (pipe_config) {
> > > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > > +			config->mode_changed = true;
> > > +
> > > +		/* Force mode sets for any infoframe stuff */
> > > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> > 
> > Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> > 
> > See https://bugs.freedesktop.org/show_bug.cgi?id=86683
> 
> Indeed. And I think the approach is the wrong way round. Imo we should
> check whether the computed pipe_config matches and have a few specific
> cases that we can handle when it mismatches (e.g. pipe_src_w/h). Plus some
> special logic if quirk flags are set (so that we force a full modeset in
> cases like this to get rid of the boot-up takever state quirk flag).
> 
> I think best course of action here is to revert this patch, also because
> it's kinda at the 1w deadline with set for regression. Even though this
> here is imo QA's fault for taking way too long to deliver the bisect
> result and noticing the issue (the patch is almost 1 month old by now).
> 
> Jesse?

I'd rather just drop the infoframe check if that's causing trouble.
The audio one should probably still stay.

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

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Daniel Vetter]

On Mon, Dec 01, 2014 at 04:37:41PM +0000, Chris Wilson wrote:
> On Mon, Dec 01, 2014 at 05:30:41PM +0100, Daniel Vetter wrote:
> > On Mon, Dec 01, 2014 at 04:09:07PM +0000, Chris Wilson wrote:
> > > On Mon, Dec 01, 2014 at 08:04:30AM -0800, Jesse Barnes wrote:
> > > > On Mon, 01 Dec 2014 12:25:45 +0200
> > > > Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > > > 
> > > > > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > > > > If these change (e.g. after a modeset following a fastboot), we need to
> > > > > > do a full mode set.
> > > > > >
> > > > > > v2:
> > > > > >   - put under pipe_config check so we don't deref a null state (Jesse)
> > > > > >
> > > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
> > > > > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > > > index cb96f11..c86eee6 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> > > > > >  						   &modeset_pipes,
> > > > > >  						   &prepare_pipes,
> > > > > >  						   &disable_pipes);
> > > > > > -	if (IS_ERR(pipe_config))
> > > > > > +	if (IS_ERR(pipe_config)) {
> > > > > >  		goto fail;
> > > > > > +	} else if (pipe_config) {
> > > > > > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > > > > > +		    to_intel_crtc(set->crtc)->config.has_audio)
> > > > > > +			config->mode_changed = true;
> > > > > > +
> > > > > > +		/* Force mode sets for any infoframe stuff */
> > > > > > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > > > > > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
> > > > > 
> > > > > Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> > > > > 
> > > > > See https://bugs.freedesktop.org/show_bug.cgi?id=86683
> > > > 
> > > > Yeah, we talked about it on IRC.  I was being extra conservative here,
> > > > since we don't yet have code to analyze whether we need a full mode set
> > > > due to something that will require an infoframe change.  Currently, the
> > > > only way for us to write a new infoframe is through a mode set, and the
> > > > old code would just happen to do a mode set most of the time we wanted
> > > > it to (i.e. when the mode timings changed), but I don't think we'd ever
> > > > thought of the implications of infoframe changes for stuff that doesn't
> > > > require a mode set.
> > > > 
> > > > We could drop this hunk and continue to play it fast & loose, but what
> > > > we really need here is a fuller check about whether we need a mode set
> > > > due to requiring an infoframe change, which means pulling them apart a
> > > > bit.
> > > 
> > > We need to be conservative. If we spot extraneous modesets in the wild,
> > > we will be motivated to fix it. As a compromise, if you include a
> > > 
> > > DRM_DEBUG_KMS("infoframes detected - forcing full modeset\n")
> > > 
> > > that should be sufficient clue for us to pick up the thread again later.
> > 
> > The problem is that with the current code we'll do a modeset for all hdmi
> > outputs, unconditionally. Even without fastboot=1 set. Which is too much
> > and a regression. E.g. just having virtual > screen and scrolling with the
> > mouse old-style would now result in a modeset each time we move one pixel.
> > Or at least that's my analysis here (didnt test).
> 
> Would it be just as easy to construct a scenario that had an infroframe
> change that didn't get applied with the revert? Besides which a full
> modeset on every pan should be good motiviation to make them nonblocking
> (or at least use vblank workers to do the clenaup) ;-)

Without fastboot we still force a full modeset at boot-up (that part
hasn't been cleanup up yet and brought out from under the fastboot
switch). So for users that don't set special options reverting this won't
cause any more modesets.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jani Nikula]

On Mon, 01 Dec 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Dec 01, 2014 at 04:37:41PM +0000, Chris Wilson wrote:
>> On Mon, Dec 01, 2014 at 05:30:41PM +0100, Daniel Vetter wrote:
>> > On Mon, Dec 01, 2014 at 04:09:07PM +0000, Chris Wilson wrote:
>> > > On Mon, Dec 01, 2014 at 08:04:30AM -0800, Jesse Barnes wrote:
>> > > > On Mon, 01 Dec 2014 12:25:45 +0200
>> > > > Jani Nikula <jani.nikula@linux.intel.com> wrote:
>> > > > 
>> > > > > On Thu, 06 Nov 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> > > > > > If these change (e.g. after a modeset following a fastboot), we need to
>> > > > > > do a full mode set.
>> > > > > >
>> > > > > > v2:
>> > > > > >   - put under pipe_config check so we don't deref a null state (Jesse)
>> > > > > >
>> > > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>> > > > > > ---
>> > > > > >  drivers/gpu/drm/i915/intel_display.c | 12 +++++++++++-
>> > > > > >  1 file changed, 11 insertions(+), 1 deletion(-)
>> > > > > >
>> > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > > > > > index cb96f11..c86eee6 100644
>> > > > > > --- a/drivers/gpu/drm/i915/intel_display.c
>> > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > > > > > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>> > > > > >  						   &modeset_pipes,
>> > > > > >  						   &prepare_pipes,
>> > > > > >  						   &disable_pipes);
>> > > > > > -	if (IS_ERR(pipe_config))
>> > > > > > +	if (IS_ERR(pipe_config)) {
>> > > > > >  		goto fail;
>> > > > > > +	} else if (pipe_config) {
>> > > > > > +		if (to_intel_crtc(set->crtc)->new_config->has_audio !=
>> > > > > > +		    to_intel_crtc(set->crtc)->config.has_audio)
>> > > > > > +			config->mode_changed = true;
>> > > > > > +
>> > > > > > +		/* Force mode sets for any infoframe stuff */
>> > > > > > +		if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
>> > > > > > +		    to_intel_crtc(set->crtc)->config.has_infoframe)
>> > > > > 
>> > > > > Jesse, is "||" correct here? This is bound to force a lot of mode sets.
>> > > > > 
>> > > > > See https://bugs.freedesktop.org/show_bug.cgi?id=86683
>> > > > 
>> > > > Yeah, we talked about it on IRC.  I was being extra conservative here,
>> > > > since we don't yet have code to analyze whether we need a full mode set
>> > > > due to something that will require an infoframe change.  Currently, the
>> > > > only way for us to write a new infoframe is through a mode set, and the
>> > > > old code would just happen to do a mode set most of the time we wanted
>> > > > it to (i.e. when the mode timings changed), but I don't think we'd ever
>> > > > thought of the implications of infoframe changes for stuff that doesn't
>> > > > require a mode set.
>> > > > 
>> > > > We could drop this hunk and continue to play it fast & loose, but what
>> > > > we really need here is a fuller check about whether we need a mode set
>> > > > due to requiring an infoframe change, which means pulling them apart a
>> > > > bit.
>> > > 
>> > > We need to be conservative. If we spot extraneous modesets in the wild,
>> > > we will be motivated to fix it. As a compromise, if you include a
>> > > 
>> > > DRM_DEBUG_KMS("infoframes detected - forcing full modeset\n")
>> > > 
>> > > that should be sufficient clue for us to pick up the thread again later.
>> > 
>> > The problem is that with the current code we'll do a modeset for all hdmi
>> > outputs, unconditionally. Even without fastboot=1 set. Which is too much
>> > and a regression. E.g. just having virtual > screen and scrolling with the
>> > mouse old-style would now result in a modeset each time we move one pixel.
>> > Or at least that's my analysis here (didnt test).
>> 
>> Would it be just as easy to construct a scenario that had an infroframe
>> change that didn't get applied with the revert? Besides which a full
>> modeset on every pan should be good motiviation to make them nonblocking
>> (or at least use vblank workers to do the clenaup) ;-)
>
> Without fastboot we still force a full modeset at boot-up (that part
> hasn't been cleanup up yet and brought out from under the fastboot
> switch). So for users that don't set special options reverting this won't
> cause any more modesets.

The bug doesn't mention any special options.

BR,
Jani.


> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

Re: [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Daniel Vetter]

On Mon, Dec 1, 2014 at 6:35 PM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>> Would it be just as easy to construct a scenario that had an infroframe
>>> change that didn't get applied with the revert? Besides which a full
>>> modeset on every pan should be good motiviation to make them nonblocking
>>> (or at least use vblank workers to do the clenaup) ;-)
>>
>> Without fastboot we still force a full modeset at boot-up (that part
>> hasn't been cleanup up yet and brought out from under the fastboot
>> switch). So for users that don't set special options reverting this won't
>> cause any more modesets.
>
> The bug doesn't mention any special options.

This was a reply to Chris' question whether there's a scenario where
reverting would cause problems, and my answer was "without fastboot=1
no". Unrelated to the bug, but related to the revert. The bug of
course happens irrespective of fastboot.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx