[PATCH 1/4] drm/i915: preserve SSC if previously set v3

[PATCH 1/4] drm/i915: preserve SSC if previously set v3

[Jesse Barnes]

Some machines may have a broken VBT or no VBT at all, but we still want
to use SSC there.  So check for it and keep it enabled if we see it
already on.  Based on an earlier fix from Kristian.

v2: honor modparam if set too (Daniel)
    read out at init time and store for panel_use_ssc() use (Jesse)
v3: trust BIOS configuration over VBT like we do for DP (Jani)

Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1363a93..3823135 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13333,6 +13333,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 
 void intel_modeset_gem_init(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *c;
 	struct drm_i915_gem_object *obj;
 
@@ -13340,6 +13341,16 @@ void intel_modeset_gem_init(struct drm_device *dev)
 	intel_init_gt_powersave(dev);
 	mutex_unlock(&dev->struct_mutex);
 
+	/*
+	 * There may be no VBT; and if the BIOS enabled SSC we can
+	 * just keep using it to avoid unnecessary flicker.  Whereas if the
+	 * BIOS isn't using it, don't assume it will work even if the VBT
+	 * indicates as much.
+	 */
+	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
+		dev_priv->vbt.lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) &
+						DREF_SSC1_ENABLE);
+
 	intel_modeset_init_hw(dev);
 
 	intel_setup_overlay(dev);
-- 
1.9.1

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

[PATCH 2/4] drm/i915: preserve swizzle settings if necessary v4

[Jesse Barnes]

Some machines (like MBAs) might use a tiled framebuffer but not enable
display swizzling at boot time.  We want to preserve that configuration
if possible to prevent a boot time mode set.  On IVB+ it shouldn't
affect performance anyway since the memory controller does internal
swizzling anyway.

For most other configs we'll be able to enable swizzling at boot time,
since the initial framebuffer won't be tiled, thus we won't see any
corruption when we enable it.

v2: preserve swizzling if BIOS had it set (Daniel)
v3: preserve swizzling only if we inherited a tiled framebuffer (Daniel)
    check display swizzle setting in detect_bit_6_swizzle (Daniel)
    use gen6 as cutoff point (Daniel)
v4: fixup swizzle preserve again, had wrong init order (Daniel)

Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.h        |  2 ++
 drivers/gpu/drm/i915/i915_gem_tiling.c | 41 +++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_display.c   |  4 ++++
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1e476b5..6bcc754 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1527,6 +1527,8 @@ struct drm_i915_private {
 	struct intel_opregion opregion;
 	struct intel_vbt_data vbt;
 
+	bool preserve_bios_swizzle;
+
 	/* overlay */
 	struct intel_overlay *overlay;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 2cefb59..d1e7a3e 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -102,22 +102,33 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
 		swizzle_x = I915_BIT_6_SWIZZLE_NONE;
 		swizzle_y = I915_BIT_6_SWIZZLE_NONE;
 	} else if (INTEL_INFO(dev)->gen >= 6) {
-		uint32_t dimm_c0, dimm_c1;
-		dimm_c0 = I915_READ(MAD_DIMM_C0);
-		dimm_c1 = I915_READ(MAD_DIMM_C1);
-		dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
-		dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
-		/* Enable swizzling when the channels are populated with
-		 * identically sized dimms. We don't need to check the 3rd
-		 * channel because no cpu with gpu attached ships in that
-		 * configuration. Also, swizzling only makes sense for 2
-		 * channels anyway. */
-		if (dimm_c0 == dimm_c1) {
-			swizzle_x = I915_BIT_6_SWIZZLE_9_10;
-			swizzle_y = I915_BIT_6_SWIZZLE_9;
+		if (dev_priv->preserve_bios_swizzle) {
+			if (I915_READ(DISP_ARB_CTL) &
+			    DISP_TILE_SURFACE_SWIZZLING) {
+				swizzle_x = I915_BIT_6_SWIZZLE_9_10;
+				swizzle_y = I915_BIT_6_SWIZZLE_9;
+			} else {
+				swizzle_x = I915_BIT_6_SWIZZLE_NONE;
+				swizzle_y = I915_BIT_6_SWIZZLE_NONE;
+			}
 		} else {
-			swizzle_x = I915_BIT_6_SWIZZLE_NONE;
-			swizzle_y = I915_BIT_6_SWIZZLE_NONE;
+			uint32_t dimm_c0, dimm_c1;
+			dimm_c0 = I915_READ(MAD_DIMM_C0);
+			dimm_c1 = I915_READ(MAD_DIMM_C1);
+			dimm_c0 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
+			dimm_c1 &= MAD_DIMM_A_SIZE_MASK | MAD_DIMM_B_SIZE_MASK;
+			/* Enable swizzling when the channels are populated
+			 * with identically sized dimms. We don't need to check
+			 * the 3rd channel because no cpu with gpu attached
+			 * ships in that configuration. Also, swizzling only
+			 * makes sense for 2 channels anyway. */
+			if (dimm_c0 == dimm_c1) {
+				swizzle_x = I915_BIT_6_SWIZZLE_9_10;
+				swizzle_y = I915_BIT_6_SWIZZLE_9;
+			} else {
+				swizzle_x = I915_BIT_6_SWIZZLE_NONE;
+				swizzle_y = I915_BIT_6_SWIZZLE_NONE;
+			}
 		}
 	} else if (IS_GEN5(dev)) {
 		/* On Ironlake whatever DRAM config, GPU always do
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3823135..1d0425d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2361,6 +2361,7 @@ static void intel_find_plane_obj(struct intel_crtc *intel_crtc,
 				 struct intel_plane_config *plane_config)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *c;
 	struct intel_crtc *i;
 	struct drm_i915_gem_object *obj;
@@ -2392,6 +2393,9 @@ static void intel_find_plane_obj(struct intel_crtc *intel_crtc,
 			continue;
 
 		if (i915_gem_obj_ggtt_offset(obj) == plane_config->base) {
+			if (obj->tiling_mode != I915_TILING_NONE)
+				dev_priv->preserve_bios_swizzle = true;
+
 			drm_framebuffer_reference(c->primary->fb);
 			intel_crtc->base.primary->fb = c->primary->fb;
 			obj->frontbuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
-- 
1.9.1

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

Re: [PATCH 2/4] drm/i915: preserve swizzle settings if necessary v4

[Daniel Vetter]

On Thu, Oct 09, 2014 at 12:57:43PM -0700, Jesse Barnes wrote:
> Some machines (like MBAs) might use a tiled framebuffer but not enable
> display swizzling at boot time.  We want to preserve that configuration
> if possible to prevent a boot time mode set.  On IVB+ it shouldn't
> affect performance anyway since the memory controller does internal
> swizzling anyway.
> 
> For most other configs we'll be able to enable swizzling at boot time,
> since the initial framebuffer won't be tiled, thus we won't see any
> corruption when we enable it.
> 
> v2: preserve swizzling if BIOS had it set (Daniel)
> v3: preserve swizzling only if we inherited a tiled framebuffer (Daniel)
>     check display swizzle setting in detect_bit_6_swizzle (Daniel)
>     use gen6 as cutoff point (Daniel)
> v4: fixup swizzle preserve again, had wrong init order (Daniel)
> 
> Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

lgtm. Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/4] drm/i915: preserve swizzle settings if necessary v4

[Jesse Barnes]

On Tue, 21 Oct 2014 16:49:12 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Oct 09, 2014 at 12:57:43PM -0700, Jesse Barnes wrote:
> > Some machines (like MBAs) might use a tiled framebuffer but not
> > enable display swizzling at boot time.  We want to preserve that
> > configuration if possible to prevent a boot time mode set.  On IVB+
> > it shouldn't affect performance anyway since the memory controller
> > does internal swizzling anyway.
> > 
> > For most other configs we'll be able to enable swizzling at boot
> > time, since the initial framebuffer won't be tiled, thus we won't
> > see any corruption when we enable it.
> > 
> > v2: preserve swizzling if BIOS had it set (Daniel)
> > v3: preserve swizzling only if we inherited a tiled framebuffer
> > (Daniel) check display swizzle setting in detect_bit_6_swizzle
> > (Daniel) use gen6 as cutoff point (Daniel)
> > v4: fixup swizzle preserve again, had wrong init order (Daniel)
> > 
> > Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> lgtm. Queued for -next, thanks for the patch.

Can you pick up the SSC one too (earlier in this thread)?  I applied
Jani's requested change which should address your concerns about the
BIOS and kernel disagreeing.

I'll ping Kristian again about the not-quite-preferred mode on his MBA.

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

[PATCH 3/4] drm: add drm_mode_same_size function

[Jesse Barnes]

From: Kristian Høgsberg <hoegsberg@gmail.com>

Like mode_equal but w/o the clock checks.  Useful for checking if modes
are close enough to re-use to avoid a boot time mode set for example.

Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/drm_modes.c | 8 ++++++++
 include/drm/drm_modes.h     | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index d1b7d20..51ede38 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -905,6 +905,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
 }
 EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
 
+bool drm_mode_same_size(const struct drm_display_mode *mode1,
+			const struct drm_display_mode *mode2)
+{
+	return mode1->vdisplay == mode2->vdisplay &&
+		mode1->hdisplay == mode2->hdisplay;
+}
+EXPORT_SYMBOL(drm_mode_same_size);
+
 /**
  * drm_mode_validate_size - make sure modes adhere to size constraints
  * @dev: DRM device
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 91d0582..7272309 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -215,6 +215,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1,
 		    const struct drm_display_mode *mode2);
 bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
 					const struct drm_display_mode *mode2);
+bool drm_mode_same_size(const struct drm_display_mode *mode1,
+			const struct drm_display_mode *mode2);
 
 /* for use by the crtc helper probe functions */
 void drm_mode_validate_size(struct drm_device *dev,
-- 
1.9.1

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

Re: [PATCH 3/4] drm: add drm_mode_same_size function

[Daniel Vetter]

On Thu, Oct 09, 2014 at 12:57:44PM -0700, Jesse Barnes wrote:
> From: Kristian Høgsberg <hoegsberg@gmail.com>
> 
> Like mode_equal but w/o the clock checks.  Useful for checking if modes
> are close enough to re-use to avoid a boot time mode set for example.
> 
> Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/drm_modes.c | 8 ++++++++
>  include/drm/drm_modes.h     | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index d1b7d20..51ede38 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -905,6 +905,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
>  }
>  EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
>  

kerneldoc is missing here.
-Daniel

> +bool drm_mode_same_size(const struct drm_display_mode *mode1,
> +			const struct drm_display_mode *mode2)
> +{
> +	return mode1->vdisplay == mode2->vdisplay &&
> +		mode1->hdisplay == mode2->hdisplay;
> +}
> +EXPORT_SYMBOL(drm_mode_same_size);
> +
>  /**
>   * drm_mode_validate_size - make sure modes adhere to size constraints
>   * @dev: DRM device
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 91d0582..7272309 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -215,6 +215,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1,
>  		    const struct drm_display_mode *mode2);
>  bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
>  					const struct drm_display_mode *mode2);
> +bool drm_mode_same_size(const struct drm_display_mode *mode1,
> +			const struct drm_display_mode *mode2);
>  
>  /* for use by the crtc helper probe functions */
>  void drm_mode_validate_size(struct drm_device *dev,
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 3/4] drm: add drm_mode_same_size function

[Jesse Barnes]

On Tue, 21 Oct 2014 16:49:37 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Oct 09, 2014 at 12:57:44PM -0700, Jesse Barnes wrote:
> > From: Kristian Høgsberg <hoegsberg@gmail.com>
> > 
> > Like mode_equal but w/o the clock checks.  Useful for checking if modes
> > are close enough to re-use to avoid a boot time mode set for example.
> > 
> > Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/drm_modes.c | 8 ++++++++
> >  include/drm/drm_modes.h     | 2 ++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index d1b7d20..51ede38 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -905,6 +905,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1,
> >  }
> >  EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
> >  
> 
> kerneldoc is missing here.

wtf I'm having deja vu here.  I can't even claim it was a failure to
"git add"... I just looked at it and then didn't do it.

-- 
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 4/4] drm/i915: use current mode if the size matches the preferred mode

[Jesse Barnes]

From: Kristian Høgsberg <hoegsberg@gmail.com>

The BIOS may set a native mode that doesn't quite match the preferred
mode timings.  It should be ok to use however if it uses the same size,
so try to avoid a mode set in that case.

Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_fbdev.c | 47 +++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 9b584f3..79dd814 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -398,42 +398,31 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
 		/* go for command line mode first */
 		modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
 
-		/* try for preferred next */
+		/* try for preferred next or match current */
 		if (!modes[i]) {
-			DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
-				      connector->name);
-			modes[i] = drm_has_preferred_mode(fb_conn, width,
-							  height);
-		}
+			struct drm_display_mode *preferred;
 
-		/* No preferred mode marked by the EDID? Are there any modes? */
-		if (!modes[i] && !list_empty(&connector->modes)) {
-			DRM_DEBUG_KMS("using first mode listed on connector %s\n",
+			DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
 				      connector->name);
-			modes[i] = list_first_entry(&connector->modes,
-						    struct drm_display_mode,
-						    head);
-		}
+			preferred = drm_has_preferred_mode(fb_conn, width,
+							   height);
 
-		/* last resort: use current mode */
-		if (!modes[i]) {
-			/*
-			 * IMPORTANT: We want to use the adjusted mode (i.e.
-			 * after the panel fitter upscaling) as the initial
-			 * config, not the input mode, which is what crtc->mode
-			 * usually contains. But since our current fastboot
-			 * code puts a mode derived from the post-pfit timings
-			 * into crtc->mode this works out correctly. We don't
-			 * use hwmode anywhere right now, so use it for this
-			 * since the fb helper layer wants a pointer to
-			 * something we own.
-			 */
-			DRM_DEBUG_KMS("looking for current mode on connector %s\n",
-				      connector->name);
 			intel_mode_from_pipe_config(&encoder->crtc->hwmode,
 						    &to_intel_crtc(encoder->crtc)->config);
-			modes[i] = &encoder->crtc->hwmode;
+ 			modes[i] = &encoder->crtc->hwmode;
+
+			if (preferred &&
+			    !drm_mode_same_size(preferred, modes[i])) {
+				DRM_DEBUG_KMS("using preferred mode %s "
+					      "instead of current mode %s "
+					      "on connector %d\n",
+					      preferred->name,
+					      modes[i]->name,
+					      fb_conn->connector->base.id);
+				modes[i] = preferred;
+			}
 		}
+
 		crtcs[i] = new_crtc;
 
 		DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
-- 
1.9.1

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

Re: [PATCH 4/4] drm/i915: use current mode if the size

[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=271/271->271/271
PNV: pass/total=269/271->269/271
ILK: pass/total=269/271->271/271
IVB: pass/total=271/271->271/271
SNB: pass/total=271/271->271/271
HSW: pass/total=271/271->271/271
BDW: pass/total=272/273->271/273
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly->result_with_patch_applied
ILK: Intel_gpu_tools, igt_kms_pipe_crc_basic_bad-nb-words-3, DMESG_WARN->PASS
ILK: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, DMESG_WARN->PASS
BDW: Intel_gpu_tools, igt_drv_missed_irq_hang, DMESG_WARN->PASS
BDW: Intel_gpu_tools, igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-forked, PASS->TIMEOUT
BDW: Intel_gpu_tools, igt_gem_concurrent_blit_gttX-bcs-gpu-read-after-write-forked, PASS->TIMEOUT

Re: [PATCH 4/4] drm/i915: use current mode if the size matches the preferred mode

[Daniel Vetter]

On Thu, Oct 09, 2014 at 12:57:45PM -0700, Jesse Barnes wrote:
> From: Kristian Høgsberg <hoegsberg@gmail.com>
> 
> The BIOS may set a native mode that doesn't quite match the preferred
> mode timings.  It should be ok to use however if it uses the same size,
> so try to avoid a mode set in that case.
> 
> Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Why exactly does this fail? Is the clock off slightly or are the timings
off?

I just wonder whether we should check vrefresh to make sure the bios
doesn't sneak a low refresh rate mode past us (from stuck drrs or
whatever).

/me thinking a bit paranoid again.

-Daniel
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 47 +++++++++++++++-----------------------
>  1 file changed, 18 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 9b584f3..79dd814 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -398,42 +398,31 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
>  		/* go for command line mode first */
>  		modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
>  
> -		/* try for preferred next */
> +		/* try for preferred next or match current */
>  		if (!modes[i]) {
> -			DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
> -				      connector->name);
> -			modes[i] = drm_has_preferred_mode(fb_conn, width,
> -							  height);
> -		}
> +			struct drm_display_mode *preferred;
>  
> -		/* No preferred mode marked by the EDID? Are there any modes? */
> -		if (!modes[i] && !list_empty(&connector->modes)) {
> -			DRM_DEBUG_KMS("using first mode listed on connector %s\n",
> +			DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
>  				      connector->name);
> -			modes[i] = list_first_entry(&connector->modes,
> -						    struct drm_display_mode,
> -						    head);
> -		}
> +			preferred = drm_has_preferred_mode(fb_conn, width,
> +							   height);
>  
> -		/* last resort: use current mode */
> -		if (!modes[i]) {
> -			/*
> -			 * IMPORTANT: We want to use the adjusted mode (i.e.
> -			 * after the panel fitter upscaling) as the initial
> -			 * config, not the input mode, which is what crtc->mode
> -			 * usually contains. But since our current fastboot
> -			 * code puts a mode derived from the post-pfit timings
> -			 * into crtc->mode this works out correctly. We don't
> -			 * use hwmode anywhere right now, so use it for this
> -			 * since the fb helper layer wants a pointer to
> -			 * something we own.
> -			 */
> -			DRM_DEBUG_KMS("looking for current mode on connector %s\n",
> -				      connector->name);
>  			intel_mode_from_pipe_config(&encoder->crtc->hwmode,
>  						    &to_intel_crtc(encoder->crtc)->config);
> -			modes[i] = &encoder->crtc->hwmode;
> + 			modes[i] = &encoder->crtc->hwmode;
> +
> +			if (preferred &&
> +			    !drm_mode_same_size(preferred, modes[i])) {
> +				DRM_DEBUG_KMS("using preferred mode %s "
> +					      "instead of current mode %s "
> +					      "on connector %d\n",
> +					      preferred->name,
> +					      modes[i]->name,
> +					      fb_conn->connector->base.id);
> +				modes[i] = preferred;
> +			}
>  		}
> +
>  		crtcs[i] = new_crtc;
>  
>  		DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 4/4] drm/i915: use current mode if the size matches the preferred mode

[Jesse Barnes]

On Tue, 21 Oct 2014 16:53:02 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Oct 09, 2014 at 12:57:45PM -0700, Jesse Barnes wrote:
> > From: Kristian Høgsberg <hoegsberg@gmail.com>
> > 
> > The BIOS may set a native mode that doesn't quite match the preferred
> > mode timings.  It should be ok to use however if it uses the same size,
> > so try to avoid a mode set in that case.
> > 
> > Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Why exactly does this fail? Is the clock off slightly or are the timings
> off?
> 
> I just wonder whether we should check vrefresh to make sure the bios
> doesn't sneak a low refresh rate mode past us (from stuck drrs or
> whatever).
> 
> /me thinking a bit paranoid again.

Yeah you asked this same thing last time.  IIRC it's not a lower clock
rate like DRRS, but rather different timings.  But Kristian will have
to post them.

Kristian, can you please post the mode lines from the EDID on this
machine and the ones the BIOS sets at boot up?

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/4] drm/i915: use current mode if the size matches the preferred mode

[Daniel Vetter]

On Wed, Oct 22, 2014 at 08:42:32AM -0700, Jesse Barnes wrote:
> On Tue, 21 Oct 2014 16:53:02 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Thu, Oct 09, 2014 at 12:57:45PM -0700, Jesse Barnes wrote:
> > > From: Kristian Høgsberg <hoegsberg@gmail.com>
> > > 
> > > The BIOS may set a native mode that doesn't quite match the preferred
> > > mode timings.  It should be ok to use however if it uses the same size,
> > > so try to avoid a mode set in that case.
> > > 
> > > Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > Why exactly does this fail? Is the clock off slightly or are the timings
> > off?
> > 
> > I just wonder whether we should check vrefresh to make sure the bios
> > doesn't sneak a low refresh rate mode past us (from stuck drrs or
> > whatever).
> > 
> > /me thinking a bit paranoid again.
> 
> Yeah you asked this same thing last time.  IIRC it's not a lower clock
> rate like DRRS, but rather different timings.  But Kristian will have
> to post them.

Yeah, I get points for being consistent! And this time around I didn't
even realize it ;-)

> Kristian, can you please post the mode lines from the EDID on this
> machine and the ones the BIOS sets at boot up?

Yeah, I think we should put the full timing data for the bios and edid
mode into the commit message. That way we can later on inspect the
situation again in case of new kinds of brokeness.

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

Re: [PATCH 1/4] drm/i915: preserve SSC if previously set v3

[Jani Nikula]

On Thu, 09 Oct 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Some machines may have a broken VBT or no VBT at all, but we still want
> to use SSC there.  So check for it and keep it enabled if we see it
> already on.  Based on an earlier fix from Kristian.
>
> v2: honor modparam if set too (Daniel)
>     read out at init time and store for panel_use_ssc() use (Jesse)
> v3: trust BIOS configuration over VBT like we do for DP (Jani)
>
> Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

In the long run I'd like to restrict changing dev_priv->vbt within
intel_bios.c only, regardless of whether that info comes from VBT or
what was set by the BIOS. We have a few exceptions here and there now,
and there's not a good place to plug this one in, really.


BR,
Jani.


> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1363a93..3823135 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13333,6 +13333,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>  
>  void intel_modeset_gem_init(struct drm_device *dev)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *c;
>  	struct drm_i915_gem_object *obj;
>  
> @@ -13340,6 +13341,16 @@ void intel_modeset_gem_init(struct drm_device *dev)
>  	intel_init_gt_powersave(dev);
>  	mutex_unlock(&dev->struct_mutex);
>  
> +	/*
> +	 * There may be no VBT; and if the BIOS enabled SSC we can
> +	 * just keep using it to avoid unnecessary flicker.  Whereas if the
> +	 * BIOS isn't using it, don't assume it will work even if the VBT
> +	 * indicates as much.
> +	 */
> +	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
> +		dev_priv->vbt.lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) &
> +						DREF_SSC1_ENABLE);
> +
>  	intel_modeset_init_hw(dev);
>  
>  	intel_setup_overlay(dev);
> -- 
> 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 1/4] drm/i915: preserve SSC if previously set v3

[Daniel Vetter]

On Thu, Nov 13, 2014 at 10:42:20AM +0200, Jani Nikula wrote:
> On Thu, 09 Oct 2014, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > Some machines may have a broken VBT or no VBT at all, but we still want
> > to use SSC there.  So check for it and keep it enabled if we see it
> > already on.  Based on an earlier fix from Kristian.
> >
> > v2: honor modparam if set too (Daniel)
> >     read out at init time and store for panel_use_ssc() use (Jesse)
> > v3: trust BIOS configuration over VBT like we do for DP (Jani)
> >
> > Reported-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Queued for -next, thanks for the patch.

> In the long run I'd like to restrict changing dev_priv->vbt within
> intel_bios.c only, regardless of whether that info comes from VBT or
> what was set by the BIOS. We have a few exceptions here and there now,
> and there's not a good place to plug this one in, really.

Yeah, otoh the entire clock tree handling on earlier pch platforms is a
bit murky overall.
-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