All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/backlight: Fix backlight takeover on LPT
@ 2018-11-20 10:39 Maarten Lankhorst
  2018-11-20 11:04 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Maarten Lankhorst @ 2018-11-20 10:39 UTC (permalink / raw)
  To: intel-gfx; +Cc: Hans de Goede, Tolga Cakir, Basil Eric Rabi

On lynxpoint the bios sometimes sets up the backlight using the CPU
display, but the driver expects using the PWM PCH override register.

Read the value from the CPU register, then convert it to the other
units by converting from the old duty cycle, to freq, to the new units.

This value is then programmed in the override register, after which
we set the override and disable the CPU display control. This allows
us to switch the source without flickering, and make the backlight
controls work in the driver.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
Cc: Basil Eric Rabi <ericbasil.rabi@gmail.com>
Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
Cc: Tolga Cakir <cevelnet@gmail.com>
Tested-by: Tolga Cakir <cevelnet@gmail.com>
---
 drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index e6cd7b55c018..3357232f1b0e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1485,7 +1485,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_panel *panel = &connector->panel;
 	u32 pch_ctl1, pch_ctl2, val;
-	bool alt;
+	bool alt, cpu_mode = false;
 
 	if (HAS_PCH_LPT(dev_priv))
 		alt = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
@@ -1507,12 +1507,40 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
 
 	panel->backlight.min = get_backlight_min_vbt(connector);
 
-	val = lpt_get_backlight(connector);
+	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
+	if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
+	    (I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE) &&
+	    !WARN_ON(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE)) {
+		u32 freq;
+
+		cpu_mode = true;
+		/*
+		 * We're in cpu mode, convert to PCH units.
+		 *
+		 * Convert CPU pwm tick back to hz, back to new PCH units again.
+		 * this is the same formula as pch_hz_to_pwm, but the other way
+		 * around..
+		 */
+		val = pch_get_backlight(connector);
+		freq = DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), val * 128);
+
+		val = lpt_hz_to_pwm(connector, freq);
+	} else
+		val = lpt_get_backlight(connector);
 	val = intel_panel_compute_brightness(connector, val);
 	panel->backlight.level = clamp(val, panel->backlight.min,
 				       panel->backlight.max);
 
-	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
+	if (cpu_mode) {
+		u32 tmp;
+
+		/* Use PWM mode, instead of cpu clock */
+		lpt_set_backlight(connector->base.state, panel->backlight.level);
+		I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
+
+		tmp = I915_READ(BLC_PWM_CPU_CTL2);
+		I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
+	}
 
 	return 0;
 }
-- 
2.19.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-20 10:39 [PATCH] drm/i915/backlight: Fix backlight takeover on LPT Maarten Lankhorst
@ 2018-11-20 11:04 ` Patchwork
  2018-11-20 11:04 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-11-20 11:04 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/backlight: Fix backlight takeover on LPT
URL   : https://patchwork.freedesktop.org/series/52746/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9b0ac137925f drm/i915/backlight: Fix backlight takeover on LPT
-:43: CHECK:BRACES: braces {} should be used on all arms of this statement
#43: FILE: drivers/gpu/drm/i915/intel_panel.c:1511:
+	if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
[...]
+	} else
[...]

-:60: CHECK:BRACES: Unbalanced braces around else statement
#60: FILE: drivers/gpu/drm/i915/intel_panel.c:1528:
+	} else

total: 0 errors, 0 warnings, 2 checks, 50 lines checked

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.SPARSE: warning for drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-20 10:39 [PATCH] drm/i915/backlight: Fix backlight takeover on LPT Maarten Lankhorst
  2018-11-20 11:04 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-11-20 11:04 ` Patchwork
  2018-11-20 11:34 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-11-20 11:04 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/backlight: Fix backlight takeover on LPT
URL   : https://patchwork.freedesktop.org/series/52746/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/backlight: Fix backlight takeover on LPT
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_panel.c:1512:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_panel.c:1531:34: warning: expression using sizeof(void)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-20 10:39 [PATCH] drm/i915/backlight: Fix backlight takeover on LPT Maarten Lankhorst
  2018-11-20 11:04 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2018-11-20 11:04 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-11-20 11:34 ` Patchwork
  2018-11-20 14:49 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-11-20 11:34 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/backlight: Fix backlight takeover on LPT
URL   : https://patchwork.freedesktop.org/series/52746/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5169 -> Patchwork_10858 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52746/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10858:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_busy@basic-flip-a:
      {fi-kbl-7567u}:     SKIP -> PASS +2

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
      {fi-kbl-7567u}:     PASS -> SKIP +27

    
== Known issues ==

  Here are the changes found in Patchwork_10858 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u2:          PASS -> DMESG-WARN (fdo#107724)

    igt@i915_module_load@reload:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      {fi-kbl-7567u}:     DMESG-WARN (fdo#103558, fdo#105602) -> PASS

    igt@gem_ctx_create@basic-files:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS
      fi-bsw-kefka:       FAIL (fdo#108656) -> PASS

    igt@gem_exec_suspend@basic-s3:
      {fi-kbl-7567u}:     DMESG-WARN (fdo#105079, fdo#103558, fdo#105602) -> PASS

    igt@i915_selftest@live_hangcheck:
      fi-bwr-2160:        DMESG-FAIL (fdo#108735) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-kbl-7567u}:     DMESG-FAIL (fdo#105079) -> SKIP

    
    ==== Warnings ====

    igt@kms_chamelium@common-hpd-after-suspend:
      {fi-kbl-7567u}:     FAIL (fdo#108755) -> DMESG-FAIL (fdo#105079)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#105079 https://bugs.freedesktop.org/show_bug.cgi?id=105079
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
  fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735
  fdo#108755 https://bugs.freedesktop.org/show_bug.cgi?id=108755


== Participating hosts (52 -> 44) ==

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u3 fi-icl-y fi-kbl-7560u 


== Build changes ==

    * Linux: CI_DRM_5169 -> Patchwork_10858

  CI_DRM_5169: ed08968fdc2130ba3cb6f3d7dbe82b6a8c39973d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4720: c27aaca295d3ca2a38521e571c012449371e4bb5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10858: 9b0ac137925fa333ba38ff5b356934929ea4a8a4 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9b0ac137925f drm/i915/backlight: Fix backlight takeover on LPT

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10858/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-20 10:39 [PATCH] drm/i915/backlight: Fix backlight takeover on LPT Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2018-11-20 11:34 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-11-20 14:49 ` Patchwork
  2018-11-20 18:51 ` [PATCH] " Ville Syrjälä
  2018-11-23 12:33 ` Jani Nikula
  5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-11-20 14:49 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/backlight: Fix backlight takeover on LPT
URL   : https://patchwork.freedesktop.org/series/52746/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5169_full -> Patchwork_10858_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10858_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10858_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10858_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          PASS -> SKIP
      shard-snb:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10858_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-bsd:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#103158) +2

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@gem_render_copy_redux@normal:
      shard-kbl:          PASS -> INCOMPLETE (fdo#106650, fdo#103665)

    igt@i915_suspend@shrink:
      {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#108784)

    igt@kms_available_modes_crc@available_mode_test_crc:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#106641)

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-hsw:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107956) +5

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#107725) +4

    igt@kms_chv_cursor_fail@pipe-c-128x128-bottom-edge:
      shard-skl:          NOTRUN -> FAIL (fdo#104671)

    igt@kms_cursor_crc@cursor-256x256-random:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#103232) +9

    igt@kms_cursor_crc@cursor-64x21-onscreen:
      shard-glk:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          PASS -> FAIL (fdo#105454)

    igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled:
      {shard-iclb}:       NOTRUN -> WARN (fdo#108336)

    igt@kms_fbcon_fbt@psr:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#107882)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-snb:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-render:
      {shard-iclb}:       PASS -> DMESG-FAIL (fdo#107724) +1

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-gtt:
      {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107724, fdo#108336)

    igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
      {shard-iclb}:       NOTRUN -> DMESG-FAIL (fdo#107724)

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-skl:          NOTRUN -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
      {shard-iclb}:       PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
      {shard-iclb}:       PASS -> DMESG-WARN (fdo#107724, fdo#108336) +6

    igt@kms_hdmi_inject@inject-audio:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#102370)

    igt@kms_mmap_write_crc:
      shard-hsw:          PASS -> DMESG-FAIL (fdo#102614)

    igt@kms_panel_fitting@legacy:
      shard-skl:          NOTRUN -> FAIL (fdo#105456)

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
      {shard-iclb}:       PASS -> DMESG-FAIL (fdo#107724, fdo#103166) +1

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@kms_plane@plane-position-covered-pipe-c-planes:
      shard-apl:          PASS -> FAIL (fdo#103166) +2

    igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +2

    igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#103166) +1

    igt@kms_psr@no_drrs:
      {shard-iclb}:       PASS -> FAIL (fdo#108341)

    igt@kms_rmfb@close-fd:
      {shard-iclb}:       NOTRUN -> DMESG-WARN (fdo#107724) +3

    igt@kms_rotation_crc@primary-rotation-180:
      shard-skl:          NOTRUN -> FAIL (fdo#103925, fdo#107815)

    igt@perf_pmu@idle-bcs0:
      shard-glk:          NOTRUN -> DMESG-WARN (fdo#106538, fdo#105763) +1

    igt@pm_backlight@basic-brightness:
      {shard-iclb}:       PASS -> DMESG-WARN (fdo#107724) +6

    igt@pm_backlight@fade_with_suspend:
      shard-skl:          NOTRUN -> FAIL (fdo#107847)

    igt@pm_rpm@universal-planes-dpms:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807) +1

    igt@pm_rps@min-max-config-loaded:
      {shard-iclb}:       NOTRUN -> FAIL (fdo#102250)

    
    ==== Possible fixes ====

    igt@gem_eio@in-flight-10ms:
      shard-glk:          FAIL (fdo#107799) -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
      shard-skl:          FAIL (fdo#108228, fdo#108470) -> PASS

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
      shard-skl:          FAIL (fdo#108470, fdo#107815) -> PASS

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-kbl:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_chv_cursor_fail@pipe-a-128x128-left-edge:
      shard-skl:          FAIL (fdo#104671) -> PASS

    igt@kms_color@pipe-c-gamma:
      shard-skl:          FAIL (fdo#108228, fdo#104782) -> PASS

    igt@kms_cursor_crc@cursor-256x256-dpms:
      shard-skl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          FAIL (fdo#103191, fdo#103232) -> PASS

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-glk:          FAIL (fdo#103232) -> PASS +1

    igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
      shard-skl:          FAIL (fdo#103184) -> PASS

    igt@kms_flip@flip-vs-absolute-wf_vblank:
      shard-apl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +17

    igt@kms_flip_tiling@flip-changes-tiling-yf:
      shard-kbl:          DMESG-WARN (fdo#105604) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
      shard-skl:          FAIL (fdo#103167) -> PASS +6

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-cpu:
      shard-skl:          FAIL (fdo#105682) -> PASS

    igt@kms_plane@plane-panning-bottom-right-pipe-c-planes:
      shard-skl:          FAIL (fdo#103166) -> PASS

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
      shard-skl:          INCOMPLETE (fdo#104108, fdo#107773) -> PASS

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          FAIL (fdo#108145, fdo#107815) -> PASS

    igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-glk:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
      shard-apl:          FAIL (fdo#103166) -> PASS +2

    igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
      {shard-iclb}:       DMESG-WARN (fdo#107724) -> PASS +1

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    igt@pm_rpm@legacy-planes:
      shard-skl:          INCOMPLETE (fdo#105959, fdo#107807) -> PASS
      {shard-iclb}:       DMESG-WARN (fdo#108654) -> PASS

    
    ==== Warnings ====

    igt@i915_suspend@shrink:
      shard-skl:          INCOMPLETE (fdo#106886) -> DMESG-WARN (fdo#108784)

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-move:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> DMESG-FAIL (fdo#106538, fdo#105763)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102370 https://bugs.freedesktop.org/show_bug.cgi?id=102370
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105456 https://bugs.freedesktop.org/show_bug.cgi?id=105456
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105604 https://bugs.freedesktop.org/show_bug.cgi?id=105604
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106650 https://bugs.freedesktop.org/show_bug.cgi?id=106650
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#107725 https://bugs.freedesktop.org/show_bug.cgi?id=107725
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107799 https://bugs.freedesktop.org/show_bug.cgi?id=107799
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107847 https://bugs.freedesktop.org/show_bug.cgi?id=107847
  fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108228 https://bugs.freedesktop.org/show_bug.cgi?id=108228
  fdo#108336 https://bugs.freedesktop.org/show_bug.cgi?id=108336
  fdo#108341 https://bugs.freedesktop.org/show_bug.cgi?id=108341
  fdo#108470 https://bugs.freedesktop.org/show_bug.cgi?id=108470
  fdo#108654 https://bugs.freedesktop.org/show_bug.cgi?id=108654
  fdo#108784 https://bugs.freedesktop.org/show_bug.cgi?id=108784
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (7 -> 7) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5169 -> Patchwork_10858

  CI_DRM_5169: ed08968fdc2130ba3cb6f3d7dbe82b6a8c39973d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4720: c27aaca295d3ca2a38521e571c012449371e4bb5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10858: 9b0ac137925fa333ba38ff5b356934929ea4a8a4 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10858/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-20 10:39 [PATCH] drm/i915/backlight: Fix backlight takeover on LPT Maarten Lankhorst
                   ` (3 preceding siblings ...)
  2018-11-20 14:49 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-11-20 18:51 ` Ville Syrjälä
  2018-11-21  7:16   ` Tolga Cakir
  2018-11-23 12:33 ` Jani Nikula
  5 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2018-11-20 18:51 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: Hans de Goede, intel-gfx, Basil Eric Rabi, Tolga Cakir

On Tue, Nov 20, 2018 at 11:39:26AM +0100, Maarten Lankhorst wrote:
> On lynxpoint the bios sometimes sets up the backlight using the CPU
> display, but the driver expects using the PWM PCH override register.
> 
> Read the value from the CPU register, then convert it to the other
> units by converting from the old duty cycle, to freq, to the new units.
> 
> This value is then programmed in the override register, after which
> we set the override and disable the CPU display control. This allows
> us to switch the source without flickering, and make the backlight
> controls work in the driver.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
> Cc: Basil Eric Rabi <ericbasil.rabi@gmail.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: Tolga Cakir <cevelnet@gmail.com>
> Tested-by: Tolga Cakir <cevelnet@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e6cd7b55c018..3357232f1b0e 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1485,7 +1485,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 pch_ctl1, pch_ctl2, val;
> -	bool alt;
> +	bool alt, cpu_mode = false;
>  
>  	if (HAS_PCH_LPT(dev_priv))
>  		alt = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
> @@ -1507,12 +1507,40 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  
>  	panel->backlight.min = get_backlight_min_vbt(connector);
>  
> -	val = lpt_get_backlight(connector);
> +	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> +	if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
> +	    (I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE) &&
> +	    !WARN_ON(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE)) {
> +		u32 freq;
> +
> +		cpu_mode = true;
> +		/*
> +		 * We're in cpu mode, convert to PCH units.
> +		 *
> +		 * Convert CPU pwm tick back to hz, back to new PCH units again.
> +		 * this is the same formula as pch_hz_to_pwm, but the other way
> +		 * around..
> +		 */
> +		val = pch_get_backlight(connector);
> +		freq = DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), val * 128);

The docs are telling me that HSW/BDW use cdclk for the CPU backlight,
and the increment is configurable as well.

Also what about S4 resume? Don't we need this trick there as well?

> +
> +		val = lpt_hz_to_pwm(connector, freq);
> +	} else
> +		val = lpt_get_backlight(connector);
>  	val = intel_panel_compute_brightness(connector, val);
>  	panel->backlight.level = clamp(val, panel->backlight.min,
>  				       panel->backlight.max);
>  
> -	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> +	if (cpu_mode) {
> +		u32 tmp;
> +
> +		/* Use PWM mode, instead of cpu clock */
> +		lpt_set_backlight(connector->base.state, panel->backlight.level);
> +		I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
> +
> +		tmp = I915_READ(BLC_PWM_CPU_CTL2);
> +		I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-20 18:51 ` [PATCH] " Ville Syrjälä
@ 2018-11-21  7:16   ` Tolga Cakir
  2018-11-21 10:45     ` Ville Syrjälä
  0 siblings, 1 reply; 10+ messages in thread
From: Tolga Cakir @ 2018-11-21  7:16 UTC (permalink / raw)
  To: ville.syrjala; +Cc: jwrdegoede, intel-gfx, ericbasil.rabi

Am Di., 20. Nov. 2018 um 19:51 Uhr schrieb Ville Syrjälä
<ville.syrjala@linux.intel.com>:
>
> On Tue, Nov 20, 2018 at 11:39:26AM +0100, Maarten Lankhorst wrote:
> > On lynxpoint the bios sometimes sets up the backlight using the CPU
> > display, but the driver expects using the PWM PCH override register.
> >
> > Read the value from the CPU register, then convert it to the other
> > units by converting from the old duty cycle, to freq, to the new units.
> >
> > This value is then programmed in the override register, after which
> > we set the override and disable the CPU display control. This allows
> > us to switch the source without flickering, and make the backlight
> > controls work in the driver.
> >
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
> > Cc: Basil Eric Rabi <ericbasil.rabi@gmail.com>
> > Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> > Cc: Tolga Cakir <cevelnet@gmail.com>
> > Tested-by: Tolga Cakir <cevelnet@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++++++---
> >  1 file changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index e6cd7b55c018..3357232f1b0e 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1485,7 +1485,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> >       struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> >       struct intel_panel *panel = &connector->panel;
> >       u32 pch_ctl1, pch_ctl2, val;
> > -     bool alt;
> > +     bool alt, cpu_mode = false;
> >
> >       if (HAS_PCH_LPT(dev_priv))
> >               alt = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
> > @@ -1507,12 +1507,40 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> >
> >       panel->backlight.min = get_backlight_min_vbt(connector);
> >
> > -     val = lpt_get_backlight(connector);
> > +     panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> > +     if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
> > +         (I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE) &&
> > +         !WARN_ON(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE)) {
> > +             u32 freq;
> > +
> > +             cpu_mode = true;
> > +             /*
> > +              * We're in cpu mode, convert to PCH units.
> > +              *
> > +              * Convert CPU pwm tick back to hz, back to new PCH units again.
> > +              * this is the same formula as pch_hz_to_pwm, but the other way
> > +              * around..
> > +              */
> > +             val = pch_get_backlight(connector);
> > +             freq = DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), val * 128);
>
> The docs are telling me that HSW/BDW use cdclk for the CPU backlight,
> and the increment is configurable as well.
>
> Also what about S4 resume? Don't we need this trick there as well?
>

Hi,

I couldn't test this patch for S4 resume on my Dell M3800 laptop (Core
i7-4702HQ, Lynx Point), as fastboot=1 doesn't work for S4 resume, only
normal boot. Therefore, there are no issues on S4 wake after
hibernate. Backlight controls etc. work as expected; just like
fastboot=0.

I have applied "[1/2] drm/i915: Enable fastset for non-boot modesets"
by Maarten, but that doesn't give me fastboot for S4 resume aswell.
Same behavior as unpatched on my system.

Please let me know, when there is more I can test.

Cheers,
Tolga

> > +
> > +             val = lpt_hz_to_pwm(connector, freq);
> > +     } else
> > +             val = lpt_get_backlight(connector);
> >       val = intel_panel_compute_brightness(connector, val);
> >       panel->backlight.level = clamp(val, panel->backlight.min,
> >                                      panel->backlight.max);
> >
> > -     panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> > +     if (cpu_mode) {
> > +             u32 tmp;
> > +
> > +             /* Use PWM mode, instead of cpu clock */
> > +             lpt_set_backlight(connector->base.state, panel->backlight.level);
> > +             I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
> > +
> > +             tmp = I915_READ(BLC_PWM_CPU_CTL2);
> > +             I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> > +     }
> >
> >       return 0;
> >  }
> > --
> > 2.19.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-21  7:16   ` Tolga Cakir
@ 2018-11-21 10:45     ` Ville Syrjälä
  2018-11-21 12:37       ` Tolga Cakir
  0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2018-11-21 10:45 UTC (permalink / raw)
  To: Tolga Cakir; +Cc: jwrdegoede, intel-gfx, ericbasil.rabi

On Wed, Nov 21, 2018 at 08:16:06AM +0100, Tolga Cakir wrote:
> Am Di., 20. Nov. 2018 um 19:51 Uhr schrieb Ville Syrjälä
> <ville.syrjala@linux.intel.com>:
> >
> > On Tue, Nov 20, 2018 at 11:39:26AM +0100, Maarten Lankhorst wrote:
> > > On lynxpoint the bios sometimes sets up the backlight using the CPU
> > > display, but the driver expects using the PWM PCH override register.
> > >
> > > Read the value from the CPU register, then convert it to the other
> > > units by converting from the old duty cycle, to freq, to the new units.
> > >
> > > This value is then programmed in the override register, after which
> > > we set the override and disable the CPU display control. This allows
> > > us to switch the source without flickering, and make the backlight
> > > controls work in the driver.
> > >
> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
> > > Cc: Basil Eric Rabi <ericbasil.rabi@gmail.com>
> > > Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> > > Cc: Tolga Cakir <cevelnet@gmail.com>
> > > Tested-by: Tolga Cakir <cevelnet@gmail.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++++++---
> > >  1 file changed, 31 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > > index e6cd7b55c018..3357232f1b0e 100644
> > > --- a/drivers/gpu/drm/i915/intel_panel.c
> > > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > > @@ -1485,7 +1485,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> > >       struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > >       struct intel_panel *panel = &connector->panel;
> > >       u32 pch_ctl1, pch_ctl2, val;
> > > -     bool alt;
> > > +     bool alt, cpu_mode = false;
> > >
> > >       if (HAS_PCH_LPT(dev_priv))
> > >               alt = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
> > > @@ -1507,12 +1507,40 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> > >
> > >       panel->backlight.min = get_backlight_min_vbt(connector);
> > >
> > > -     val = lpt_get_backlight(connector);
> > > +     panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> > > +     if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
> > > +         (I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE) &&
> > > +         !WARN_ON(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE)) {
> > > +             u32 freq;
> > > +
> > > +             cpu_mode = true;
> > > +             /*
> > > +              * We're in cpu mode, convert to PCH units.
> > > +              *
> > > +              * Convert CPU pwm tick back to hz, back to new PCH units again.
> > > +              * this is the same formula as pch_hz_to_pwm, but the other way
> > > +              * around..
> > > +              */
> > > +             val = pch_get_backlight(connector);
> > > +             freq = DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), val * 128);
> >
> > The docs are telling me that HSW/BDW use cdclk for the CPU backlight,
> > and the increment is configurable as well.
> >
> > Also what about S4 resume? Don't we need this trick there as well?
> >
> 
> Hi,
> 
> I couldn't test this patch for S4 resume on my Dell M3800 laptop (Core
> i7-4702HQ, Lynx Point), as fastboot=1 doesn't work for S4 resume, only
> normal boot. Therefore, there are no issues on S4 wake after
> hibernate. Backlight controls etc. work as expected; just like
> fastboot=0.

I think fastboot should work the same for s4 resume as boot, but
only if your boot kernel doesn't load i915.ko. That way the
resuming kernel will directly take over from the BIOS programmed
display configuration. OTOH if the boot kernel loads i915 then
it will shut down all displays before handing control over to
the resuming kernel.

> 
> I have applied "[1/2] drm/i915: Enable fastset for non-boot modesets"
> by Maarten, but that doesn't give me fastboot for S4 resume aswell.
> Same behavior as unpatched on my system.
> 
> Please let me know, when there is more I can test.
> 
> Cheers,
> Tolga
> 
> > > +
> > > +             val = lpt_hz_to_pwm(connector, freq);
> > > +     } else
> > > +             val = lpt_get_backlight(connector);
> > >       val = intel_panel_compute_brightness(connector, val);
> > >       panel->backlight.level = clamp(val, panel->backlight.min,
> > >                                      panel->backlight.max);
> > >
> > > -     panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> > > +     if (cpu_mode) {
> > > +             u32 tmp;
> > > +
> > > +             /* Use PWM mode, instead of cpu clock */
> > > +             lpt_set_backlight(connector->base.state, panel->backlight.level);
> > > +             I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
> > > +
> > > +             tmp = I915_READ(BLC_PWM_CPU_CTL2);
> > > +             I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> > > +     }
> > >
> > >       return 0;
> > >  }
> > > --
> > > 2.19.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-21 10:45     ` Ville Syrjälä
@ 2018-11-21 12:37       ` Tolga Cakir
  0 siblings, 0 replies; 10+ messages in thread
From: Tolga Cakir @ 2018-11-21 12:37 UTC (permalink / raw)
  To: ville.syrjala; +Cc: Hans De Goede, intel-gfx, Basil Eric Rabi

Am Mi., 21. Nov. 2018 um 11:45 Uhr schrieb Ville Syrjälä
<ville.syrjala@linux.intel.com>:
>
> On Wed, Nov 21, 2018 at 08:16:06AM +0100, Tolga Cakir wrote:
> > Am Di., 20. Nov. 2018 um 19:51 Uhr schrieb Ville Syrjälä
> > <ville.syrjala@linux.intel.com>:
> > >
> > > On Tue, Nov 20, 2018 at 11:39:26AM +0100, Maarten Lankhorst wrote:
> > > > On lynxpoint the bios sometimes sets up the backlight using the CPU
> > > > display, but the driver expects using the PWM PCH override register.
> > > >
> > > > Read the value from the CPU register, then convert it to the other
> > > > units by converting from the old duty cycle, to freq, to the new units.
> > > >
> > > > This value is then programmed in the override register, after which
> > > > we set the override and disable the CPU display control. This allows
> > > > us to switch the source without flickering, and make the backlight
> > > > controls work in the driver.
> > > >
> > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
> > > > Cc: Basil Eric Rabi <ericbasil.rabi@gmail.com>
> > > > Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> > > > Cc: Tolga Cakir <cevelnet@gmail.com>
> > > > Tested-by: Tolga Cakir <cevelnet@gmail.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++++++---
> > > >  1 file changed, 31 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > > > index e6cd7b55c018..3357232f1b0e 100644
> > > > --- a/drivers/gpu/drm/i915/intel_panel.c
> > > > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > > > @@ -1485,7 +1485,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> > > >       struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > > >       struct intel_panel *panel = &connector->panel;
> > > >       u32 pch_ctl1, pch_ctl2, val;
> > > > -     bool alt;
> > > > +     bool alt, cpu_mode = false;
> > > >
> > > >       if (HAS_PCH_LPT(dev_priv))
> > > >               alt = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
> > > > @@ -1507,12 +1507,40 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
> > > >
> > > >       panel->backlight.min = get_backlight_min_vbt(connector);
> > > >
> > > > -     val = lpt_get_backlight(connector);
> > > > +     panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> > > > +     if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
> > > > +         (I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE) &&
> > > > +         !WARN_ON(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE)) {
> > > > +             u32 freq;
> > > > +
> > > > +             cpu_mode = true;
> > > > +             /*
> > > > +              * We're in cpu mode, convert to PCH units.
> > > > +              *
> > > > +              * Convert CPU pwm tick back to hz, back to new PCH units again.
> > > > +              * this is the same formula as pch_hz_to_pwm, but the other way
> > > > +              * around..
> > > > +              */
> > > > +             val = pch_get_backlight(connector);
> > > > +             freq = DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), val * 128);
> > >
> > > The docs are telling me that HSW/BDW use cdclk for the CPU backlight,
> > > and the increment is configurable as well.
> > >
> > > Also what about S4 resume? Don't we need this trick there as well?
> > >
> >
> > Hi,
> >
> > I couldn't test this patch for S4 resume on my Dell M3800 laptop (Core
> > i7-4702HQ, Lynx Point), as fastboot=1 doesn't work for S4 resume, only
> > normal boot. Therefore, there are no issues on S4 wake after
> > hibernate. Backlight controls etc. work as expected; just like
> > fastboot=0.
>
> I think fastboot should work the same for s4 resume as boot, but
> only if your boot kernel doesn't load i915.ko. That way the
> resuming kernel will directly take over from the BIOS programmed
> display configuration. OTOH if the boot kernel loads i915 then
> it will shut down all displays before handing control over to
> the resuming kernel.
>

You're right, fastboot=1 worked for S4 resume, it was GNOME doing the
modeset after S4 resume for whatever reason. I have disabled GNOME for
further testing. Just hibernated from console and S4 resumed back to
console: no flickering. As you already suspected, backlight controls
didn't work, eventhough they worked on normal boot with fastboot=1.
So, this patch also needs to be applied to S4 resume, as you said.
Good catch!

I'm ready to test as soon as the patches are ready.

> >
> > I have applied "[1/2] drm/i915: Enable fastset for non-boot modesets"
> > by Maarten, but that doesn't give me fastboot for S4 resume aswell.
> > Same behavior as unpatched on my system.
> >
> > Please let me know, when there is more I can test.
> >
> > Cheers,
> > Tolga
> >
> > > > +
> > > > +             val = lpt_hz_to_pwm(connector, freq);
> > > > +     } else
> > > > +             val = lpt_get_backlight(connector);
> > > >       val = intel_panel_compute_brightness(connector, val);
> > > >       panel->backlight.level = clamp(val, panel->backlight.min,
> > > >                                      panel->backlight.max);
> > > >
> > > > -     panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> > > > +     if (cpu_mode) {
> > > > +             u32 tmp;
> > > > +
> > > > +             /* Use PWM mode, instead of cpu clock */
> > > > +             lpt_set_backlight(connector->base.state, panel->backlight.level);
> > > > +             I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
> > > > +
> > > > +             tmp = I915_READ(BLC_PWM_CPU_CTL2);
> > > > +             I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> > > > +     }
> > > >
> > > >       return 0;
> > > >  }
> > > > --
> > > > 2.19.1
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/backlight: Fix backlight takeover on LPT
  2018-11-20 10:39 [PATCH] drm/i915/backlight: Fix backlight takeover on LPT Maarten Lankhorst
                   ` (4 preceding siblings ...)
  2018-11-20 18:51 ` [PATCH] " Ville Syrjälä
@ 2018-11-23 12:33 ` Jani Nikula
  5 siblings, 0 replies; 10+ messages in thread
From: Jani Nikula @ 2018-11-23 12:33 UTC (permalink / raw)
  To: Maarten Lankhorst, intel-gfx; +Cc: Hans de Goede, Basil Eric Rabi, Tolga Cakir

On Tue, 20 Nov 2018, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> On lynxpoint the bios sometimes sets up the backlight using the CPU
> display, but the driver expects using the PWM PCH override register.
>
> Read the value from the CPU register, then convert it to the other
> units by converting from the old duty cycle, to freq, to the new units.
>
> This value is then programmed in the override register, after which
> we set the override and disable the CPU display control. This allows
> us to switch the source without flickering, and make the backlight
> controls work in the driver.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108225
> Cc: Basil Eric Rabi <ericbasil.rabi@gmail.com>
> Cc: Hans de Goede <jwrdegoede@fedoraproject.org>
> Cc: Tolga Cakir <cevelnet@gmail.com>
> Tested-by: Tolga Cakir <cevelnet@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 34 +++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index e6cd7b55c018..3357232f1b0e 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1485,7 +1485,7 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_panel *panel = &connector->panel;
>  	u32 pch_ctl1, pch_ctl2, val;
> -	bool alt;
> +	bool alt, cpu_mode = false;
>  
>  	if (HAS_PCH_LPT(dev_priv))
>  		alt = I915_READ(SOUTH_CHICKEN2) & LPT_PWM_GRANULARITY;
> @@ -1507,12 +1507,40 @@ static int lpt_setup_backlight(struct intel_connector *connector, enum pipe unus
>  
>  	panel->backlight.min = get_backlight_min_vbt(connector);
>  
> -	val = lpt_get_backlight(connector);
> +	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> +	if (panel->backlight.enabled && HAS_PCH_LPT(dev_priv) &&
> +	    (I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE) &&
> +	    !WARN_ON(pch_ctl1 & BLM_PCH_OVERRIDE_ENABLE)) {

I think there's something wrong with the conditions here, but can't
quite wrap my head around it.

At least it seems wrong to warn about override enable in this case. If
override is set, it'll be used. So the warning should really be about
cpu_ctl2 being enabled when it's not used.

Perhaps read cpu_ctl2 into a variable, and reuse that also later for
disable.

BR,
Jani.

> +		u32 freq;
> +
> +		cpu_mode = true;
> +		/*
> +		 * We're in cpu mode, convert to PCH units.
> +		 *
> +		 * Convert CPU pwm tick back to hz, back to new PCH units again.
> +		 * this is the same formula as pch_hz_to_pwm, but the other way
> +		 * around..
> +		 */
> +		val = pch_get_backlight(connector);
> +		freq = DIV_ROUND_CLOSEST(KHz(dev_priv->rawclk_freq), val * 128);
> +
> +		val = lpt_hz_to_pwm(connector, freq);
> +	} else
> +		val = lpt_get_backlight(connector);
>  	val = intel_panel_compute_brightness(connector, val);
>  	panel->backlight.level = clamp(val, panel->backlight.min,
>  				       panel->backlight.max);
>  
> -	panel->backlight.enabled = pch_ctl1 & BLM_PCH_PWM_ENABLE;
> +	if (cpu_mode) {
> +		u32 tmp;
> +
> +		/* Use PWM mode, instead of cpu clock */
> +		lpt_set_backlight(connector->base.state, panel->backlight.level);
> +		I915_WRITE(BLC_PWM_PCH_CTL1, pch_ctl1 | BLM_PCH_OVERRIDE_ENABLE);
> +
> +		tmp = I915_READ(BLC_PWM_CPU_CTL2);
> +		I915_WRITE(BLC_PWM_CPU_CTL2, tmp & ~BLM_PWM_ENABLE);
> +	}
>  
>  	return 0;
>  }

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-11-23 12:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-20 10:39 [PATCH] drm/i915/backlight: Fix backlight takeover on LPT Maarten Lankhorst
2018-11-20 11:04 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-11-20 11:04 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-11-20 11:34 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-20 14:49 ` ✓ Fi.CI.IGT: " Patchwork
2018-11-20 18:51 ` [PATCH] " Ville Syrjälä
2018-11-21  7:16   ` Tolga Cakir
2018-11-21 10:45     ` Ville Syrjälä
2018-11-21 12:37       ` Tolga Cakir
2018-11-23 12:33 ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.