[PATCH v2 0/5] Faster modeset support.

[PATCH v2 0/5] Faster modeset support.

[Maarten Lankhorst]

What was previously known as fastboot no longer works. But we can do much better now
that we pass the old crtc state to begin_crtc_commit.

Even if we decide not to enable modeset this patch is useful, because it cleans up
some non-atomic bits in a good way.

Maarten Lankhorst (5):
  drm/i915: Set csc coefficients in update_pipe_size.
  drm/i915: Remove references to crtc->active from intel_fbdev.c
  drm/i915: Always try to inherit the initial fb.
  drm/i915: Make updating pipe without modeset atomic.
  drm/i915: skip modeset if compatible for everyone.

 drivers/gpu/drm/i915/i915_drv.h      |   1 -
 drivers/gpu/drm/i915/i915_params.c   |   5 --
 drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
 drivers/gpu/drm/i915/intel_display.c | 116 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 drivers/gpu/drm/i915/intel_fbdev.c   |   9 +--
 6 files changed, 79 insertions(+), 56 deletions(-)

-- 
2.1.0

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

[PATCH v2 1/5] drm/i915: Set csc coefficients in update_pipe_size.

[Maarten Lankhorst]

This might not have been set during boot, and when we preserve
the initial mode this can result in a black screen.

Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 364104dee32f..7b5dfe2f7b2d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3314,6 +3314,9 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
 	if (!i915.fastboot)
 		return;
 
+	if (HAS_DDI(dev))
+		intel_set_pipe_csc(&crtc->base);
+
 	/*
 	 * 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
-- 
2.1.0

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

[PATCH v2 2/5] drm/i915: Remove references to crtc->active from intel_fbdev.c

[Maarten Lankhorst]

It should really use the atomic state.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 6c9351b2e3af..6333241624c4 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -587,7 +587,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 			intel_fb_obj(crtc->primary->state->fb);
 		intel_crtc = to_intel_crtc(crtc);
 
-		if (!intel_crtc->active || !obj) {
+		if (!crtc->state->active || !obj) {
 			DRM_DEBUG_KMS("pipe %c not active or no fb, skipping\n",
 				      pipe_name(intel_crtc->pipe));
 			continue;
@@ -612,7 +612,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 
 		intel_crtc = to_intel_crtc(crtc);
 
-		if (!intel_crtc->active) {
+		if (!crtc->state->active) {
 			DRM_DEBUG_KMS("pipe %c not active, skipping\n",
 				      pipe_name(intel_crtc->pipe));
 			continue;
@@ -675,7 +675,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 	for_each_crtc(dev, crtc) {
 		intel_crtc = to_intel_crtc(crtc);
 
-		if (!intel_crtc->active)
+		if (!crtc->state->active)
 			continue;
 
 		WARN(!crtc->primary->fb,
-- 
2.1.0

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

[PATCH v2 3/5] drm/i915: Always try to inherit the initial fb.

[Maarten Lankhorst]

The initial state is read out correctly and the state is atomic,
so it's safe to preserve the fb without any hacks if it's suitable.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 6333241624c4..179c2c37c1b0 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -578,9 +578,6 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
 	struct intel_crtc *intel_crtc;
 	unsigned int max_size = 0;
 
-	if (!i915.fastboot)
-		return false;
-
 	/* Find the largest fb */
 	for_each_crtc(dev, crtc) {
 		struct drm_i915_gem_object *obj =
-- 
2.1.0

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

[PATCH v2 4/5] drm/i915: Make updating pipe without modeset atomic.

[Maarten Lankhorst]

Instead of doing a hack during primary plane commit the state
is updated during atomic evasion. It handles differences in
pipe size and the panel fitter.

This is continuing on top of Daniel's work to make faster
modesets atomic, and not yet enabled by default.

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

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 9336e8030980..8287b81287a0 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -98,8 +98,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 		return NULL;
 
 	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
-
 	crtc_state->base.crtc = crtc;
+	crtc_state->update_pipe = false;
 
 	return &crtc_state->base;
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7b5dfe2f7b2d..d3874a68cdb9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -108,6 +108,9 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
 	struct intel_crtc_state *crtc_state);
 static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
 			   int num_connectors);
+static void skylake_pfit_enable(struct intel_crtc *crtc);
+static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
+static void ironlake_pfit_enable(struct intel_crtc *crtc);
 static void intel_modeset_setup_hw_state(struct drm_device *dev);
 
 typedef struct {
@@ -3305,14 +3308,20 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
 	return pending;
 }
 
-static void intel_update_pipe_size(struct intel_crtc *crtc)
+static void intel_update_pipe_config(struct intel_crtc *crtc,
+				     struct intel_crtc_state *old_crtc_state)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	const struct drm_display_mode *adjusted_mode;
+	struct intel_crtc_state *pipe_config =
+		to_intel_crtc_state(crtc->base.state);
 
-	if (!i915.fastboot)
-		return;
+	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
+	crtc->base.mode = crtc->base.state->mode;
+
+	DRM_DEBUG_KMS("Updating pipe size %ix%i -> %ix%i\n",
+		      old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
+		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
 
 	if (HAS_DDI(dev))
 		intel_set_pipe_csc(&crtc->base);
@@ -3324,27 +3333,26 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
 	 * 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.
 	 */
 
-	adjusted_mode = &crtc->config->base.adjusted_mode;
-
 	I915_WRITE(PIPESRC(crtc->pipe),
-		   ((adjusted_mode->crtc_hdisplay - 1) << 16) |
-		   (adjusted_mode->crtc_vdisplay - 1));
-	if (!crtc->config->pch_pfit.enabled &&
-	    (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
-	     intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
-		I915_WRITE(PF_CTL(crtc->pipe), 0);
-		I915_WRITE(PF_WIN_POS(crtc->pipe), 0);
-		I915_WRITE(PF_WIN_SZ(crtc->pipe), 0);
+		   ((pipe_config->pipe_src_w - 1) << 16) |
+		   (pipe_config->pipe_src_h - 1));
+
+	/* on skylake this is done by detaching scalers */
+	if (INTEL_INFO(dev)->gen == 9) {
+		skl_detach_scalers(crtc);
+
+		if (pipe_config->pch_pfit.enabled)
+			skylake_pfit_enable(crtc);
+	}
+	else if (INTEL_INFO(dev)->gen < 9 &&
+	         HAS_PCH_SPLIT(dev)) {
+		if (pipe_config->pch_pfit.enabled)
+			ironlake_pfit_enable(crtc);
+		else if (old_crtc_state->pch_pfit.enabled)
+			ironlake_pfit_disable(crtc, true);
 	}
-	crtc->config->pipe_src_w = adjusted_mode->crtc_hdisplay;
-	crtc->config->pipe_src_h = adjusted_mode->crtc_vdisplay;
 }
 
 static void intel_fdi_normal_train(struct drm_crtc *crtc)
@@ -5003,7 +5011,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	}
 }
 
-static void ironlake_pfit_disable(struct intel_crtc *crtc)
+static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5011,7 +5019,7 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc)
 
 	/* To avoid upsetting the power well on haswell only disable the pfit if
 	 * it's in use. The hw state code will make sure we get this right. */
-	if (crtc->config->pch_pfit.enabled) {
+	if (force || crtc->config->pch_pfit.enabled) {
 		I915_WRITE(PF_CTL(pipe), 0);
 		I915_WRITE(PF_WIN_POS(pipe), 0);
 		I915_WRITE(PF_WIN_SZ(pipe), 0);
@@ -5038,7 +5046,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 
 	intel_disable_pipe(intel_crtc);
 
-	ironlake_pfit_disable(intel_crtc);
+	ironlake_pfit_disable(intel_crtc, false);
 
 	if (intel_crtc->config->has_pch_encoder)
 		ironlake_fdi_disable(crtc);
@@ -5101,7 +5109,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (INTEL_INFO(dev)->gen == 9)
 		skylake_scaler_disable(intel_crtc);
 	else if (INTEL_INFO(dev)->gen < 9)
-		ironlake_pfit_disable(intel_crtc);
+		ironlake_pfit_disable(intel_crtc, false);
 	else
 		MISSING_CASE(INTEL_INFO(dev)->gen);
 
@@ -12246,7 +12254,6 @@ static bool intel_fuzzy_clock_check(int clock1, int clock2)
 			    base.head) \
 		if (mask & (1 <<(intel_crtc)->pipe))
 
-
 static bool
 intel_compare_m_n(unsigned int m, unsigned int n,
 		  unsigned int m2, unsigned int n2,
@@ -12467,8 +12474,16 @@ intel_pipe_config_compare(struct drm_device *dev,
 				      DRM_MODE_FLAG_NVSYNC);
 	}
 
-	PIPE_CONF_CHECK_I(pipe_src_w);
-	PIPE_CONF_CHECK_I(pipe_src_h);
+	if (!adjust) {
+		PIPE_CONF_CHECK_I(pipe_src_w);
+		PIPE_CONF_CHECK_I(pipe_src_h);
+
+		PIPE_CONF_CHECK_I(pch_pfit.enabled);
+		if (current_config->pch_pfit.enabled) {
+			PIPE_CONF_CHECK_I(pch_pfit.pos);
+			PIPE_CONF_CHECK_I(pch_pfit.size);
+		}
+	}
 
 	PIPE_CONF_CHECK_I(gmch_pfit.control);
 	/* pfit ratios are autocomputed by the hw on gen4+ */
@@ -12476,12 +12491,6 @@ intel_pipe_config_compare(struct drm_device *dev,
 		PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
 	PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
 
-	PIPE_CONF_CHECK_I(pch_pfit.enabled);
-	if (current_config->pch_pfit.enabled) {
-		PIPE_CONF_CHECK_I(pch_pfit.pos);
-		PIPE_CONF_CHECK_I(pch_pfit.size);
-	}
-
 	PIPE_CONF_CHECK_I(scaler_state.scaler_id);
 
 	/* BDW+ don't expose a synchronous way to read the state */
@@ -12644,7 +12653,8 @@ check_crtc_state(struct drm_device *dev, struct drm_atomic_state *old_state)
 		struct intel_crtc_state *pipe_config, *sw_config;
 		bool active;
 
-		if (!needs_modeset(crtc->state))
+		if (!needs_modeset(crtc->state) &&
+		    !to_intel_crtc_state(crtc->state)->update_pipe)
 			continue;
 
 		__drm_atomic_helper_crtc_destroy_state(crtc, old_crtc_state);
@@ -12940,7 +12950,6 @@ static int intel_modeset_all_pipes(struct drm_atomic_state *state)
 	return ret;
 }
 
-
 static int intel_modeset_checks(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
@@ -13031,6 +13040,7 @@ static int intel_atomic_check(struct drm_device *dev,
 					to_intel_crtc_state(crtc->state),
 					pipe_config, true)) {
 			crtc_state->mode_changed = false;
+			to_intel_crtc_state(crtc_state)->update_pipe = true;
 		}
 
 		if (needs_modeset(crtc_state)) {
@@ -13128,16 +13138,30 @@ static int intel_atomic_commit(struct drm_device *dev,
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		bool modeset = needs_modeset(crtc->state);
+		bool update_pipe = !modeset &&
+			to_intel_crtc_state(crtc->state)->update_pipe;
+		unsigned long put_domains = 0;
 
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
 			dev_priv->display.crtc_enable(crtc);
 		}
 
+		if (update_pipe) {
+			put_domains = modeset_get_crtc_power_domains(crtc);
+
+			/* make sure intel_modeset_check_state runs */
+			any_ms = true;
+		}
+
 		if (!modeset)
 			intel_pre_plane_update(intel_crtc);
 
 		drm_atomic_helper_commit_planes_on_crtc(crtc_state);
+
+		if (put_domains)
+			modeset_put_power_domains(dev_priv, put_domains);
+
 		intel_post_plane_update(intel_crtc);
 	}
 
@@ -13454,10 +13478,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	if (!crtc->state->active)
 		return;
 
-	if (state->visible)
-		/* FIXME: kill this fastboot hack */
-		intel_update_pipe_size(intel_crtc);
-
 	dev_priv->display.update_primary_plane(crtc, fb, crtc->x, crtc->y);
 }
 
@@ -13476,6 +13496,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *old_intel_state =
+		to_intel_crtc_state(old_crtc_state);
+	bool modeset = needs_modeset(crtc->state);
 
 	if (intel_crtc->atomic.update_wm_pre)
 		intel_update_watermarks(crtc);
@@ -13484,7 +13507,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 	if (crtc->state->active)
 		intel_pipe_update_start(intel_crtc, &intel_crtc->start_vbl_count);
 
-	if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
+	if (modeset)
+		return;
+
+	if (to_intel_crtc_state(crtc->state)->update_pipe)
+		intel_update_pipe_config(intel_crtc, old_intel_state);
+	else if (INTEL_INFO(dev)->gen >= 9)
 		skl_detach_scalers(intel_crtc);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 22dc8b6b4198..73b254bcd451 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -337,6 +337,8 @@ struct intel_crtc_state {
 #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS	(1<<0) /* unreliable sync mode.flags */
 	unsigned long quirks;
 
+	bool update_pipe;
+
 	/* Pipe source size (ie. panel fitter input size)
 	 * All planes will be positioned inside this space,
 	 * and get clipped at the edges. */
-- 
2.1.0

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

[PATCH v2 5/5] drm/i915: skip modeset if compatible for everyone.

[Maarten Lankhorst]

This is done as a separate commit, to make it easier to revert
when things break.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      | 1 -
 drivers/gpu/drm/i915/i915_params.c   | 5 -----
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 3 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 61244455347f..c75d343ff26b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2628,7 +2628,6 @@ struct i915_params {
 	int enable_cmd_parser;
 	/* leave bools at the end to not create holes */
 	bool enable_hangcheck;
-	bool fastboot;
 	bool prefault_disable;
 	bool load_detect_test;
 	bool reset;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 5ae4b0aba564..47d41da9d942 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -40,7 +40,6 @@ struct i915_params i915 __read_mostly = {
 	.preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
 	.disable_power_well = 1,
 	.enable_ips = 1,
-	.fastboot = 0,
 	.prefault_disable = 0,
 	.load_detect_test = 0,
 	.reset = true,
@@ -132,10 +131,6 @@ MODULE_PARM_DESC(disable_power_well,
 module_param_named(enable_ips, i915.enable_ips, int, 0600);
 MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
 
-module_param_named(fastboot, i915.fastboot, bool, 0600);
-MODULE_PARM_DESC(fastboot,
-	"Try to skip unnecessary mode sets at boot time (default: false)");
-
 module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600);
 MODULE_PARM_DESC(prefault_disable,
 	"Disable page prefaulting for pread/pwrite/reloc (default:false). "
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d3874a68cdb9..b4fb5dec2e38 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13035,8 +13035,7 @@ static int intel_atomic_check(struct drm_device *dev,
 		if (ret)
 			return ret;
 
-		if (i915.fastboot &&
-		    intel_pipe_config_compare(state->dev,
+		if (intel_pipe_config_compare(state->dev,
 					to_intel_crtc_state(crtc->state),
 					pipe_config, true)) {
 			crtc_state->mode_changed = false;
-- 
2.1.0

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

Re: [PATCH v2 4/5] drm/i915: Make updating pipe without modeset atomic.

[Ville Syrjälä]

On Thu, Aug 27, 2015 at 03:44:05PM +0200, Maarten Lankhorst wrote:
> Instead of doing a hack during primary plane commit the state
> is updated during atomic evasion. It handles differences in
> pipe size and the panel fitter.
> 
> This is continuing on top of Daniel's work to make faster
> modesets atomic, and not yet enabled by default.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
>  drivers/gpu/drm/i915/intel_display.c | 110 ++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>  3 files changed, 72 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 9336e8030980..8287b81287a0 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -98,8 +98,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>  		return NULL;
>  
>  	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
> -
>  	crtc_state->base.crtc = crtc;
> +	crtc_state->update_pipe = false;
>  
>  	return &crtc_state->base;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7b5dfe2f7b2d..d3874a68cdb9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -108,6 +108,9 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
>  	struct intel_crtc_state *crtc_state);
>  static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
>  			   int num_connectors);
> +static void skylake_pfit_enable(struct intel_crtc *crtc);
> +static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> +static void ironlake_pfit_enable(struct intel_crtc *crtc);
>  static void intel_modeset_setup_hw_state(struct drm_device *dev);
>  
>  typedef struct {
> @@ -3305,14 +3308,20 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
>  	return pending;
>  }
>  
> -static void intel_update_pipe_size(struct intel_crtc *crtc)
> +static void intel_update_pipe_config(struct intel_crtc *crtc,
> +				     struct intel_crtc_state *old_crtc_state)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	const struct drm_display_mode *adjusted_mode;
> +	struct intel_crtc_state *pipe_config =
> +		to_intel_crtc_state(crtc->base.state);
>  
> -	if (!i915.fastboot)
> -		return;
> +	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
> +	crtc->base.mode = crtc->base.state->mode;
> +
> +	DRM_DEBUG_KMS("Updating pipe size %ix%i -> %ix%i\n",
> +		      old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
> +		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>  
>  	if (HAS_DDI(dev))
>  		intel_set_pipe_csc(&crtc->base);
> @@ -3324,27 +3333,26 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
>  	 * 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.
>  	 */
>  
> -	adjusted_mode = &crtc->config->base.adjusted_mode;
> -
>  	I915_WRITE(PIPESRC(crtc->pipe),
> -		   ((adjusted_mode->crtc_hdisplay - 1) << 16) |
> -		   (adjusted_mode->crtc_vdisplay - 1));
> -	if (!crtc->config->pch_pfit.enabled &&
> -	    (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
> -	     intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
> -		I915_WRITE(PF_CTL(crtc->pipe), 0);
> -		I915_WRITE(PF_WIN_POS(crtc->pipe), 0);
> -		I915_WRITE(PF_WIN_SZ(crtc->pipe), 0);
> +		   ((pipe_config->pipe_src_w - 1) << 16) |
> +		   (pipe_config->pipe_src_h - 1));
> +
> +	/* on skylake this is done by detaching scalers */
> +	if (INTEL_INFO(dev)->gen == 9) {
> +		skl_detach_scalers(crtc);
> +
> +		if (pipe_config->pch_pfit.enabled)
> +			skylake_pfit_enable(crtc);
> +	}
> +	else if (INTEL_INFO(dev)->gen < 9 &&
> +	         HAS_PCH_SPLIT(dev)) {
> +		if (pipe_config->pch_pfit.enabled)
> +			ironlake_pfit_enable(crtc);
> +		else if (old_crtc_state->pch_pfit.enabled)
> +			ironlake_pfit_disable(crtc, true);
>  	}
> -	crtc->config->pipe_src_w = adjusted_mode->crtc_hdisplay;
> -	crtc->config->pipe_src_h = adjusted_mode->crtc_vdisplay;
>  }
>  
>  static void intel_fdi_normal_train(struct drm_crtc *crtc)
> @@ -5003,7 +5011,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  	}
>  }
>  
> -static void ironlake_pfit_disable(struct intel_crtc *crtc)
> +static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5011,7 +5019,7 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc)
>  
>  	/* To avoid upsetting the power well on haswell only disable the pfit if
>  	 * it's in use. The hw state code will make sure we get this right. */
> -	if (crtc->config->pch_pfit.enabled) {
> +	if (force || crtc->config->pch_pfit.enabled) {
>  		I915_WRITE(PF_CTL(pipe), 0);
>  		I915_WRITE(PF_WIN_POS(pipe), 0);
>  		I915_WRITE(PF_WIN_SZ(pipe), 0);
> @@ -5038,7 +5046,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  
>  	intel_disable_pipe(intel_crtc);
>  
> -	ironlake_pfit_disable(intel_crtc);
> +	ironlake_pfit_disable(intel_crtc, false);
>  
>  	if (intel_crtc->config->has_pch_encoder)
>  		ironlake_fdi_disable(crtc);
> @@ -5101,7 +5109,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  	if (INTEL_INFO(dev)->gen == 9)
>  		skylake_scaler_disable(intel_crtc);
>  	else if (INTEL_INFO(dev)->gen < 9)
> -		ironlake_pfit_disable(intel_crtc);
> +		ironlake_pfit_disable(intel_crtc, false);
>  	else
>  		MISSING_CASE(INTEL_INFO(dev)->gen);
>  
> @@ -12246,7 +12254,6 @@ static bool intel_fuzzy_clock_check(int clock1, int clock2)
>  			    base.head) \
>  		if (mask & (1 <<(intel_crtc)->pipe))
>  
> -
>  static bool
>  intel_compare_m_n(unsigned int m, unsigned int n,
>  		  unsigned int m2, unsigned int n2,
> @@ -12467,8 +12474,16 @@ intel_pipe_config_compare(struct drm_device *dev,
>  				      DRM_MODE_FLAG_NVSYNC);
>  	}
>  
> -	PIPE_CONF_CHECK_I(pipe_src_w);
> -	PIPE_CONF_CHECK_I(pipe_src_h);
> +	if (!adjust) {
> +		PIPE_CONF_CHECK_I(pipe_src_w);
> +		PIPE_CONF_CHECK_I(pipe_src_h);
> +
> +		PIPE_CONF_CHECK_I(pch_pfit.enabled);
> +		if (current_config->pch_pfit.enabled) {
> +			PIPE_CONF_CHECK_I(pch_pfit.pos);
> +			PIPE_CONF_CHECK_I(pch_pfit.size);
> +		}
> +	}

What's this hack now? Aren't you computing the correct config or why do
you need to skip the state check?

>  
>  	PIPE_CONF_CHECK_I(gmch_pfit.control);
>  	/* pfit ratios are autocomputed by the hw on gen4+ */
> @@ -12476,12 +12491,6 @@ intel_pipe_config_compare(struct drm_device *dev,
>  		PIPE_CONF_CHECK_I(gmch_pfit.pgm_ratios);
>  	PIPE_CONF_CHECK_I(gmch_pfit.lvds_border_bits);
>  
> -	PIPE_CONF_CHECK_I(pch_pfit.enabled);
> -	if (current_config->pch_pfit.enabled) {
> -		PIPE_CONF_CHECK_I(pch_pfit.pos);
> -		PIPE_CONF_CHECK_I(pch_pfit.size);
> -	}
> -
>  	PIPE_CONF_CHECK_I(scaler_state.scaler_id);
>  
>  	/* BDW+ don't expose a synchronous way to read the state */
> @@ -12644,7 +12653,8 @@ check_crtc_state(struct drm_device *dev, struct drm_atomic_state *old_state)
>  		struct intel_crtc_state *pipe_config, *sw_config;
>  		bool active;
>  
> -		if (!needs_modeset(crtc->state))
> +		if (!needs_modeset(crtc->state) &&
> +		    !to_intel_crtc_state(crtc->state)->update_pipe)
>  			continue;
>  
>  		__drm_atomic_helper_crtc_destroy_state(crtc, old_crtc_state);
> @@ -12940,7 +12950,6 @@ static int intel_modeset_all_pipes(struct drm_atomic_state *state)
>  	return ret;
>  }
>  
> -
>  static int intel_modeset_checks(struct drm_atomic_state *state)
>  {
>  	struct drm_device *dev = state->dev;
> @@ -13031,6 +13040,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  					to_intel_crtc_state(crtc->state),
>  					pipe_config, true)) {
>  			crtc_state->mode_changed = false;
> +			to_intel_crtc_state(crtc_state)->update_pipe = true;
>  		}
>  
>  		if (needs_modeset(crtc_state)) {
> @@ -13128,16 +13138,30 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  		bool modeset = needs_modeset(crtc->state);
> +		bool update_pipe = !modeset &&
> +			to_intel_crtc_state(crtc->state)->update_pipe;
> +		unsigned long put_domains = 0;
>  
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
>  			dev_priv->display.crtc_enable(crtc);
>  		}
>  
> +		if (update_pipe) {
> +			put_domains = modeset_get_crtc_power_domains(crtc);
> +
> +			/* make sure intel_modeset_check_state runs */
> +			any_ms = true;
> +		}
> +
>  		if (!modeset)
>  			intel_pre_plane_update(intel_crtc);
>  
>  		drm_atomic_helper_commit_planes_on_crtc(crtc_state);
> +
> +		if (put_domains)
> +			modeset_put_power_domains(dev_priv, put_domains);
> +
>  		intel_post_plane_update(intel_crtc);
>  	}
>  
> @@ -13454,10 +13478,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
>  	if (!crtc->state->active)
>  		return;
>  
> -	if (state->visible)
> -		/* FIXME: kill this fastboot hack */
> -		intel_update_pipe_size(intel_crtc);
> -
>  	dev_priv->display.update_primary_plane(crtc, fb, crtc->x, crtc->y);
>  }
>  
> @@ -13476,6 +13496,9 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc_state *old_intel_state =
> +		to_intel_crtc_state(old_crtc_state);
> +	bool modeset = needs_modeset(crtc->state);
>  
>  	if (intel_crtc->atomic.update_wm_pre)
>  		intel_update_watermarks(crtc);
> @@ -13484,7 +13507,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
>  	if (crtc->state->active)
>  		intel_pipe_update_start(intel_crtc, &intel_crtc->start_vbl_count);
>  
> -	if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9)
> +	if (modeset)
> +		return;
> +
> +	if (to_intel_crtc_state(crtc->state)->update_pipe)
> +		intel_update_pipe_config(intel_crtc, old_intel_state);
> +	else if (INTEL_INFO(dev)->gen >= 9)
>  		skl_detach_scalers(intel_crtc);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 22dc8b6b4198..73b254bcd451 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -337,6 +337,8 @@ struct intel_crtc_state {
>  #define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS	(1<<0) /* unreliable sync mode.flags */
>  	unsigned long quirks;
>  
> +	bool update_pipe;
> +
>  	/* Pipe source size (ie. panel fitter input size)
>  	 * All planes will be positioned inside this space,
>  	 * and get clipped at the edges. */
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v2 4/5] drm/i915: Make updating pipe without modeset atomic.

[Maarten Lankhorst]

Op 27-08-15 om 15:58 schreef Ville Syrjälä:
> On Thu, Aug 27, 2015 at 03:44:05PM +0200, Maarten Lankhorst wrote:
>> Instead of doing a hack during primary plane commit the state
>> is updated during atomic evasion. It handles differences in
>> pipe size and the panel fitter.
>>
>> This is continuing on top of Daniel's work to make faster
>> modesets atomic, and not yet enabled by default.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
>>  drivers/gpu/drm/i915/intel_display.c | 110 ++++++++++++++++++++++-------------
>>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
>>  3 files changed, 72 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
>> index 9336e8030980..8287b81287a0 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -98,8 +98,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>>  		return NULL;
>>  
>>  	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
>> -
>>  	crtc_state->base.crtc = crtc;
>> +	crtc_state->update_pipe = false;
>>  
>>  	return &crtc_state->base;
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 7b5dfe2f7b2d..d3874a68cdb9 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -108,6 +108,9 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
>>  	struct intel_crtc_state *crtc_state);
>>  static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
>>  			   int num_connectors);
>> +static void skylake_pfit_enable(struct intel_crtc *crtc);
>> +static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
>> +static void ironlake_pfit_enable(struct intel_crtc *crtc);
>>  static void intel_modeset_setup_hw_state(struct drm_device *dev);
>>  
>>  typedef struct {
>> @@ -3305,14 +3308,20 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
>>  	return pending;
>>  }
>>  
>> -static void intel_update_pipe_size(struct intel_crtc *crtc)
>> +static void intel_update_pipe_config(struct intel_crtc *crtc,
>> +				     struct intel_crtc_state *old_crtc_state)
>>  {
>>  	struct drm_device *dev = crtc->base.dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	const struct drm_display_mode *adjusted_mode;
>> +	struct intel_crtc_state *pipe_config =
>> +		to_intel_crtc_state(crtc->base.state);
>>  
>> -	if (!i915.fastboot)
>> -		return;
>> +	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
>> +	crtc->base.mode = crtc->base.state->mode;
>> +
>> +	DRM_DEBUG_KMS("Updating pipe size %ix%i -> %ix%i\n",
>> +		      old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
>> +		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>>  
>>  	if (HAS_DDI(dev))
>>  		intel_set_pipe_csc(&crtc->base);
>> @@ -3324,27 +3333,26 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
>>  	 * 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.
>>  	 */
>>  
>> -	adjusted_mode = &crtc->config->base.adjusted_mode;
>> -
>>  	I915_WRITE(PIPESRC(crtc->pipe),
>> -		   ((adjusted_mode->crtc_hdisplay - 1) << 16) |
>> -		   (adjusted_mode->crtc_vdisplay - 1));
>> -	if (!crtc->config->pch_pfit.enabled &&
>> -	    (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
>> -	     intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
>> -		I915_WRITE(PF_CTL(crtc->pipe), 0);
>> -		I915_WRITE(PF_WIN_POS(crtc->pipe), 0);
>> -		I915_WRITE(PF_WIN_SZ(crtc->pipe), 0);
>> +		   ((pipe_config->pipe_src_w - 1) << 16) |
>> +		   (pipe_config->pipe_src_h - 1));
>> +
>> +	/* on skylake this is done by detaching scalers */
>> +	if (INTEL_INFO(dev)->gen == 9) {
>> +		skl_detach_scalers(crtc);
>> +
>> +		if (pipe_config->pch_pfit.enabled)
>> +			skylake_pfit_enable(crtc);
>> +	}
>> +	else if (INTEL_INFO(dev)->gen < 9 &&
>> +	         HAS_PCH_SPLIT(dev)) {
>> +		if (pipe_config->pch_pfit.enabled)
>> +			ironlake_pfit_enable(crtc);
>> +		else if (old_crtc_state->pch_pfit.enabled)
>> +			ironlake_pfit_disable(crtc, true);
>>  	}
>> -	crtc->config->pipe_src_w = adjusted_mode->crtc_hdisplay;
>> -	crtc->config->pipe_src_h = adjusted_mode->crtc_vdisplay;
>>  }
>>  
>>  static void intel_fdi_normal_train(struct drm_crtc *crtc)
>> @@ -5003,7 +5011,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>>  	}
>>  }
>>  
>> -static void ironlake_pfit_disable(struct intel_crtc *crtc)
>> +static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
>>  {
>>  	struct drm_device *dev = crtc->base.dev;
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -5011,7 +5019,7 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc)
>>  
>>  	/* To avoid upsetting the power well on haswell only disable the pfit if
>>  	 * it's in use. The hw state code will make sure we get this right. */
>> -	if (crtc->config->pch_pfit.enabled) {
>> +	if (force || crtc->config->pch_pfit.enabled) {
>>  		I915_WRITE(PF_CTL(pipe), 0);
>>  		I915_WRITE(PF_WIN_POS(pipe), 0);
>>  		I915_WRITE(PF_WIN_SZ(pipe), 0);
>> @@ -5038,7 +5046,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>>  
>>  	intel_disable_pipe(intel_crtc);
>>  
>> -	ironlake_pfit_disable(intel_crtc);
>> +	ironlake_pfit_disable(intel_crtc, false);
>>  
>>  	if (intel_crtc->config->has_pch_encoder)
>>  		ironlake_fdi_disable(crtc);
>> @@ -5101,7 +5109,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>>  	if (INTEL_INFO(dev)->gen == 9)
>>  		skylake_scaler_disable(intel_crtc);
>>  	else if (INTEL_INFO(dev)->gen < 9)
>> -		ironlake_pfit_disable(intel_crtc);
>> +		ironlake_pfit_disable(intel_crtc, false);
>>  	else
>>  		MISSING_CASE(INTEL_INFO(dev)->gen);
>>  
>> @@ -12246,7 +12254,6 @@ static bool intel_fuzzy_clock_check(int clock1, int clock2)
>>  			    base.head) \
>>  		if (mask & (1 <<(intel_crtc)->pipe))
>>  
>> -
>>  static bool
>>  intel_compare_m_n(unsigned int m, unsigned int n,
>>  		  unsigned int m2, unsigned int n2,
>> @@ -12467,8 +12474,16 @@ intel_pipe_config_compare(struct drm_device *dev,
>>  				      DRM_MODE_FLAG_NVSYNC);
>>  	}
>>  
>> -	PIPE_CONF_CHECK_I(pipe_src_w);
>> -	PIPE_CONF_CHECK_I(pipe_src_h);
>> +	if (!adjust) {
>> +		PIPE_CONF_CHECK_I(pipe_src_w);
>> +		PIPE_CONF_CHECK_I(pipe_src_h);
>> +
>> +		PIPE_CONF_CHECK_I(pch_pfit.enabled);
>> +		if (current_config->pch_pfit.enabled) {
>> +			PIPE_CONF_CHECK_I(pch_pfit.pos);
>> +			PIPE_CONF_CHECK_I(pch_pfit.size);
>> +		}
>> +	}
> What's this hack now? Aren't you computing the correct config or why do
> you need to skip the state check?
>
This is not a hack but intentional. This function is called with adjust=true when comparing the state
to see if a full blown modeset is needed or not. Because  intel_update_pipe_config can modify
pch_pfit and pipe_src* there's no need to consider it when doing a modeset.

But you do need the true values, or intel_update_pipe_config will do the wrong thing.

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

Re: [PATCH v2 4/5] drm/i915: Make updating pipe without modeset atomic.

[Ville Syrjälä]

On Thu, Aug 27, 2015 at 04:06:46PM +0200, Maarten Lankhorst wrote:
> Op 27-08-15 om 15:58 schreef Ville Syrjälä:
> > On Thu, Aug 27, 2015 at 03:44:05PM +0200, Maarten Lankhorst wrote:
> >> Instead of doing a hack during primary plane commit the state
> >> is updated during atomic evasion. It handles differences in
> >> pipe size and the panel fitter.
> >>
> >> This is continuing on top of Daniel's work to make faster
> >> modesets atomic, and not yet enabled by default.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
> >>  drivers/gpu/drm/i915/intel_display.c | 110 ++++++++++++++++++++++-------------
> >>  drivers/gpu/drm/i915/intel_drv.h     |   2 +
> >>  3 files changed, 72 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> >> index 9336e8030980..8287b81287a0 100644
> >> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >> @@ -98,8 +98,8 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
> >>  		return NULL;
> >>  
> >>  	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
> >> -
> >>  	crtc_state->base.crtc = crtc;
> >> +	crtc_state->update_pipe = false;
> >>  
> >>  	return &crtc_state->base;
> >>  }
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 7b5dfe2f7b2d..d3874a68cdb9 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -108,6 +108,9 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
> >>  	struct intel_crtc_state *crtc_state);
> >>  static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
> >>  			   int num_connectors);
> >> +static void skylake_pfit_enable(struct intel_crtc *crtc);
> >> +static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> >> +static void ironlake_pfit_enable(struct intel_crtc *crtc);
> >>  static void intel_modeset_setup_hw_state(struct drm_device *dev);
> >>  
> >>  typedef struct {
> >> @@ -3305,14 +3308,20 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
> >>  	return pending;
> >>  }
> >>  
> >> -static void intel_update_pipe_size(struct intel_crtc *crtc)
> >> +static void intel_update_pipe_config(struct intel_crtc *crtc,
> >> +				     struct intel_crtc_state *old_crtc_state)
> >>  {
> >>  	struct drm_device *dev = crtc->base.dev;
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >> -	const struct drm_display_mode *adjusted_mode;
> >> +	struct intel_crtc_state *pipe_config =
> >> +		to_intel_crtc_state(crtc->base.state);
> >>  
> >> -	if (!i915.fastboot)
> >> -		return;
> >> +	/* drm_atomic_helper_update_legacy_modeset_state might not be called. */
> >> +	crtc->base.mode = crtc->base.state->mode;
> >> +
> >> +	DRM_DEBUG_KMS("Updating pipe size %ix%i -> %ix%i\n",
> >> +		      old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
> >> +		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
> >>  
> >>  	if (HAS_DDI(dev))
> >>  		intel_set_pipe_csc(&crtc->base);
> >> @@ -3324,27 +3333,26 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
> >>  	 * 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.
> >>  	 */
> >>  
> >> -	adjusted_mode = &crtc->config->base.adjusted_mode;
> >> -
> >>  	I915_WRITE(PIPESRC(crtc->pipe),
> >> -		   ((adjusted_mode->crtc_hdisplay - 1) << 16) |
> >> -		   (adjusted_mode->crtc_vdisplay - 1));
> >> -	if (!crtc->config->pch_pfit.enabled &&
> >> -	    (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
> >> -	     intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
> >> -		I915_WRITE(PF_CTL(crtc->pipe), 0);
> >> -		I915_WRITE(PF_WIN_POS(crtc->pipe), 0);
> >> -		I915_WRITE(PF_WIN_SZ(crtc->pipe), 0);
> >> +		   ((pipe_config->pipe_src_w - 1) << 16) |
> >> +		   (pipe_config->pipe_src_h - 1));
> >> +
> >> +	/* on skylake this is done by detaching scalers */
> >> +	if (INTEL_INFO(dev)->gen == 9) {
> >> +		skl_detach_scalers(crtc);
> >> +
> >> +		if (pipe_config->pch_pfit.enabled)
> >> +			skylake_pfit_enable(crtc);
> >> +	}
> >> +	else if (INTEL_INFO(dev)->gen < 9 &&
> >> +	         HAS_PCH_SPLIT(dev)) {
> >> +		if (pipe_config->pch_pfit.enabled)
> >> +			ironlake_pfit_enable(crtc);
> >> +		else if (old_crtc_state->pch_pfit.enabled)
> >> +			ironlake_pfit_disable(crtc, true);
> >>  	}
> >> -	crtc->config->pipe_src_w = adjusted_mode->crtc_hdisplay;
> >> -	crtc->config->pipe_src_h = adjusted_mode->crtc_vdisplay;
> >>  }
> >>  
> >>  static void intel_fdi_normal_train(struct drm_crtc *crtc)
> >> @@ -5003,7 +5011,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >>  	}
> >>  }
> >>  
> >> -static void ironlake_pfit_disable(struct intel_crtc *crtc)
> >> +static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
> >>  {
> >>  	struct drm_device *dev = crtc->base.dev;
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >> @@ -5011,7 +5019,7 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc)
> >>  
> >>  	/* To avoid upsetting the power well on haswell only disable the pfit if
> >>  	 * it's in use. The hw state code will make sure we get this right. */
> >> -	if (crtc->config->pch_pfit.enabled) {
> >> +	if (force || crtc->config->pch_pfit.enabled) {
> >>  		I915_WRITE(PF_CTL(pipe), 0);
> >>  		I915_WRITE(PF_WIN_POS(pipe), 0);
> >>  		I915_WRITE(PF_WIN_SZ(pipe), 0);
> >> @@ -5038,7 +5046,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> >>  
> >>  	intel_disable_pipe(intel_crtc);
> >>  
> >> -	ironlake_pfit_disable(intel_crtc);
> >> +	ironlake_pfit_disable(intel_crtc, false);
> >>  
> >>  	if (intel_crtc->config->has_pch_encoder)
> >>  		ironlake_fdi_disable(crtc);
> >> @@ -5101,7 +5109,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
> >>  	if (INTEL_INFO(dev)->gen == 9)
> >>  		skylake_scaler_disable(intel_crtc);
> >>  	else if (INTEL_INFO(dev)->gen < 9)
> >> -		ironlake_pfit_disable(intel_crtc);
> >> +		ironlake_pfit_disable(intel_crtc, false);
> >>  	else
> >>  		MISSING_CASE(INTEL_INFO(dev)->gen);
> >>  
> >> @@ -12246,7 +12254,6 @@ static bool intel_fuzzy_clock_check(int clock1, int clock2)
> >>  			    base.head) \
> >>  		if (mask & (1 <<(intel_crtc)->pipe))
> >>  
> >> -
> >>  static bool
> >>  intel_compare_m_n(unsigned int m, unsigned int n,
> >>  		  unsigned int m2, unsigned int n2,
> >> @@ -12467,8 +12474,16 @@ intel_pipe_config_compare(struct drm_device *dev,
> >>  				      DRM_MODE_FLAG_NVSYNC);
> >>  	}
> >>  
> >> -	PIPE_CONF_CHECK_I(pipe_src_w);
> >> -	PIPE_CONF_CHECK_I(pipe_src_h);
> >> +	if (!adjust) {
> >> +		PIPE_CONF_CHECK_I(pipe_src_w);
> >> +		PIPE_CONF_CHECK_I(pipe_src_h);
> >> +
> >> +		PIPE_CONF_CHECK_I(pch_pfit.enabled);
> >> +		if (current_config->pch_pfit.enabled) {
> >> +			PIPE_CONF_CHECK_I(pch_pfit.pos);
> >> +			PIPE_CONF_CHECK_I(pch_pfit.size);
> >> +		}
> >> +	}
> > What's this hack now? Aren't you computing the correct config or why do
> > you need to skip the state check?
> >
> This is not a hack but intentional. This function is called with adjust=true when comparing the state
> to see if a full blown modeset is needed or not. Because  intel_update_pipe_config can modify
> pch_pfit and pipe_src* there's no need to consider it when doing a modeset.
> 
> But you do need the true values, or intel_update_pipe_config will do the wrong thing.

Ah. I didn't realize we'd given it a dual purpose. Does make some sense
though to avoid having write similar tests twice.

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

Re: [PATCH v2 5/5] drm/i915: skip modeset if compatible for everyone.

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7277
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              253/253              252/253
SNB                                  248/248              248/248
IVB                                  281/281              281/281
BYT                 -2              234/234              232/234
HSW                                  317/317              317/317
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@gem_reloc_vs_gpu@forked-interruptible-faulting-reloc-thrashing      PASS(1)      DMESG_WARN(1)
*BYT  igt@gem_partial_pwrite_pread@reads-display      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/5] drm/i915: Set csc coefficients in update_pipe_size.

[Jesse Barnes]

On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
> This might not have been set during boot, and when we preserve
> the initial mode this can result in a black screen.
> 
> Cc: Daniel Stone <daniels@collabora.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 364104dee32f..7b5dfe2f7b2d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3314,6 +3314,9 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
>  	if (!i915.fastboot)
>  		return;
>  
> +	if (HAS_DDI(dev))
> +		intel_set_pipe_csc(&crtc->base);
> +
>  	/*
>  	 * 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
> 

Maybe there's a better place to put this later?

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/5] drm/i915: Remove references to crtc->active from intel_fbdev.c

[Jesse Barnes]

On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
> It should really use the atomic state.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 6c9351b2e3af..6333241624c4 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -587,7 +587,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>  			intel_fb_obj(crtc->primary->state->fb);
>  		intel_crtc = to_intel_crtc(crtc);
>  
> -		if (!intel_crtc->active || !obj) {
> +		if (!crtc->state->active || !obj) {
>  			DRM_DEBUG_KMS("pipe %c not active or no fb, skipping\n",
>  				      pipe_name(intel_crtc->pipe));
>  			continue;
> @@ -612,7 +612,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>  
>  		intel_crtc = to_intel_crtc(crtc);
>  
> -		if (!intel_crtc->active) {
> +		if (!crtc->state->active) {
>  			DRM_DEBUG_KMS("pipe %c not active, skipping\n",
>  				      pipe_name(intel_crtc->pipe));
>  			continue;
> @@ -675,7 +675,7 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>  	for_each_crtc(dev, crtc) {
>  		intel_crtc = to_intel_crtc(crtc);
>  
> -		if (!intel_crtc->active)
> +		if (!crtc->state->active)
>  			continue;
>  
>  		WARN(!crtc->primary->fb,
> 

We still have too many of these "active" states to track...  Maybe we should rename one or more.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/5] drm/i915: Always try to inherit the initial fb.

[Jesse Barnes]

On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
> The initial state is read out correctly and the state is atomic,
> so it's safe to preserve the fb without any hacks if it's suitable.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 6333241624c4..179c2c37c1b0 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -578,9 +578,6 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>  	struct intel_crtc *intel_crtc;
>  	unsigned int max_size = 0;
>  
> -	if (!i915.fastboot)
> -		return false;
> -
>  	/* Find the largest fb */
>  	for_each_crtc(dev, crtc) {
>  		struct drm_i915_gem_object *obj =
> 

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/5] drm/i915: Set csc coefficients in update_pipe_size.

[Jesse Barnes]

On 09/10/2015 12:25 PM, Jesse Barnes wrote:
> On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
>> This might not have been set during boot, and when we preserve
>> the initial mode this can result in a black screen.
>>
>> Cc: Daniel Stone <daniels@collabora.com>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 364104dee32f..7b5dfe2f7b2d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3314,6 +3314,9 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
>>  	if (!i915.fastboot)
>>  		return;
>>  
>> +	if (HAS_DDI(dev))
>> +		intel_set_pipe_csc(&crtc->base);
>> +
>>  	/*
>>  	 * 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
>>
> 
> Maybe there's a better place to put this later?

Nevermind, I see the function gets renamed later; much clearer.

Jesse

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

Re: [PATCH v2 4/5] drm/i915: Make updating pipe without modeset atomic.

[Jesse Barnes]

On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
> +	/* on skylake this is done by detaching scalers */
> +	if (INTEL_INFO(dev)->gen == 9) {
> +		skl_detach_scalers(crtc);
> +
> +		if (pipe_config->pch_pfit.enabled)
> +			skylake_pfit_enable(crtc);
> +	}
> +	else if (INTEL_INFO(dev)->gen < 9 &&
> +	         HAS_PCH_SPLIT(dev)) {
> +		if (pipe_config->pch_pfit.enabled)
> +			ironlake_pfit_enable(crtc);
> +		else if (old_crtc_state->pch_pfit.enabled)
> +			ironlake_pfit_disable(crtc, true);
>  	}

Funky coding style, but otherwise looks ok...

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915: skip modeset if compatible for everyone.

[Jesse Barnes]

On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
> This is done as a separate commit, to make it easier to revert
> when things break.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      | 1 -
>  drivers/gpu/drm/i915/i915_params.c   | 5 -----
>  drivers/gpu/drm/i915/intel_display.c | 3 +--
>  3 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 61244455347f..c75d343ff26b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2628,7 +2628,6 @@ struct i915_params {
>  	int enable_cmd_parser;
>  	/* leave bools at the end to not create holes */
>  	bool enable_hangcheck;
> -	bool fastboot;
>  	bool prefault_disable;
>  	bool load_detect_test;
>  	bool reset;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 5ae4b0aba564..47d41da9d942 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -40,7 +40,6 @@ struct i915_params i915 __read_mostly = {
>  	.preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
>  	.disable_power_well = 1,
>  	.enable_ips = 1,
> -	.fastboot = 0,
>  	.prefault_disable = 0,
>  	.load_detect_test = 0,
>  	.reset = true,
> @@ -132,10 +131,6 @@ MODULE_PARM_DESC(disable_power_well,
>  module_param_named(enable_ips, i915.enable_ips, int, 0600);
>  MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
>  
> -module_param_named(fastboot, i915.fastboot, bool, 0600);
> -MODULE_PARM_DESC(fastboot,
> -	"Try to skip unnecessary mode sets at boot time (default: false)");
> -
>  module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600);
>  MODULE_PARM_DESC(prefault_disable,
>  	"Disable page prefaulting for pread/pwrite/reloc (default:false). "
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d3874a68cdb9..b4fb5dec2e38 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13035,8 +13035,7 @@ static int intel_atomic_check(struct drm_device *dev,
>  		if (ret)
>  			return ret;
>  
> -		if (i915.fastboot &&
> -		    intel_pipe_config_compare(state->dev,
> +		if (intel_pipe_config_compare(state->dev,
>  					to_intel_crtc_state(crtc->state),
>  					pipe_config, true)) {
>  			crtc_state->mode_changed = false;
> 

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Now we just need to test it everywhere...
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/5] drm/i915: Make updating pipe without modeset atomic.

[Daniel Vetter]

On Thu, Sep 10, 2015 at 12:31:03PM -0700, Jesse Barnes wrote:
> On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
> > +	/* on skylake this is done by detaching scalers */
> > +	if (INTEL_INFO(dev)->gen == 9) {
> > +		skl_detach_scalers(crtc);
> > +
> > +		if (pipe_config->pch_pfit.enabled)
> > +			skylake_pfit_enable(crtc);
> > +	}
> > +	else if (INTEL_INFO(dev)->gen < 9 &&
> > +	         HAS_PCH_SPLIT(dev)) {
> > +		if (pipe_config->pch_pfit.enabled)
> > +			ironlake_pfit_enable(crtc);
> > +		else if (old_crtc_state->pch_pfit.enabled)
> > +			ironlake_pfit_disable(crtc, true);
> >  	}
> 
> Funky coding style, but otherwise looks ok...

Yeah I simplified this and future-proved it to make the skl_* be called
for gen9+.

But I did spot that the check for scaler_id in the pipe config compare
function is not in the !adjust block ... does fastboot on skl actually
work?

Thanks, Daniel

> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH v2 5/5] drm/i915: skip modeset if compatible for everyone.

[Daniel Vetter]

On Thu, Sep 10, 2015 at 12:31:33PM -0700, Jesse Barnes wrote:
> On 08/27/2015 06:44 AM, Maarten Lankhorst wrote:
> > This is done as a separate commit, to make it easier to revert
> > when things break.
> > 
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      | 1 -
> >  drivers/gpu/drm/i915/i915_params.c   | 5 -----
> >  drivers/gpu/drm/i915/intel_display.c | 3 +--
> >  3 files changed, 1 insertion(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 61244455347f..c75d343ff26b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2628,7 +2628,6 @@ struct i915_params {
> >  	int enable_cmd_parser;
> >  	/* leave bools at the end to not create holes */
> >  	bool enable_hangcheck;
> > -	bool fastboot;
> >  	bool prefault_disable;
> >  	bool load_detect_test;
> >  	bool reset;
> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > index 5ae4b0aba564..47d41da9d942 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -40,7 +40,6 @@ struct i915_params i915 __read_mostly = {
> >  	.preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
> >  	.disable_power_well = 1,
> >  	.enable_ips = 1,
> > -	.fastboot = 0,
> >  	.prefault_disable = 0,
> >  	.load_detect_test = 0,
> >  	.reset = true,
> > @@ -132,10 +131,6 @@ MODULE_PARM_DESC(disable_power_well,
> >  module_param_named(enable_ips, i915.enable_ips, int, 0600);
> >  MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
> >  
> > -module_param_named(fastboot, i915.fastboot, bool, 0600);
> > -MODULE_PARM_DESC(fastboot,
> > -	"Try to skip unnecessary mode sets at boot time (default: false)");
> > -
> >  module_param_named_unsafe(prefault_disable, i915.prefault_disable, bool, 0600);
> >  MODULE_PARM_DESC(prefault_disable,
> >  	"Disable page prefaulting for pread/pwrite/reloc (default:false). "
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d3874a68cdb9..b4fb5dec2e38 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13035,8 +13035,7 @@ static int intel_atomic_check(struct drm_device *dev,
> >  		if (ret)
> >  			return ret;
> >  
> > -		if (i915.fastboot &&
> > -		    intel_pipe_config_compare(state->dev,
> > +		if (intel_pipe_config_compare(state->dev,
> >  					to_intel_crtc_state(crtc->state),
> >  					pipe_config, true)) {
> >  			crtc_state->mode_changed = false;
> > 
> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Entire series applied, thanks for patches&review.

> Now we just need to test it everywhere...

On that topic: The igt testcase for this is still mia. There's a JIRA for
it, but atm JIRA is down :( Main gist that I think we need to check is
fast modesets between different upscaled and native versions. Later on we
can extend it (e.g. for userspace-controlled DRRS I want to use this same
fast modeset infrastructure here too).

Who's going to do that testcase?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx