mbox series

[v4,00/15] acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API

Message ID 20200708211432.28612-1-hdegoede@redhat.com (mailing list archive)
Headers show
Series acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API | expand

Message

Hans de Goede July 8, 2020, 9:14 p.m. UTC
Hi All,

Here is v4 of my patch series converting the i915 driver's code for
controlling the panel's backlight with an external PWM controller to
use the atomic PWM API. See below for the changelog.

Initially the plan was for this series to consist of 2 parts:
1. convert the pwm-crc driver to support the atomic PWM API and
2. convert the i915 driver's PWM code to use the atomic PWM API.

But during testing I've found a number of bugs in the pwm-lpss and I
found that the acpi_lpss code needs some special handling because of
some ugliness found in most Cherry Trail DSDTs.

So now this series has grown somewhat large and consists of 4 parts:

1. acpi_lpss fixes workarounds for Cherry Trail DSTD nastiness
2. various fixes to the pwm-lpss driver
3. convert the pwm-crc driver to support the atomic PWM API and
4. convert the i915 driver's PWM code to use the atomic PWM API

The involved acpi_lpss and pwm drivers do not see a whole lot of churn,
so the plan is to merge this all through drm-intel-next-queued (dinq)
once all the patches are reviewed / have acks.

In v4 the ACPI patches have been Acked by Rafael and the i915 patches
have been acked by Jani. So that just leaves the PWM patches.

Uwe can I get your ok / ack for merging this through the dinq branch
once you have acked al the PWM patches ?

This series has been tested (and re-tested after adding various bug-fixes)
extensively. It has been tested on the following devices:

-Asus T100TA  BYT + CRC-PMIC PWM
-Toshiba WT8-A  BYT + CRC-PMIC PWM
-Thundersoft TS178 BYT + CRC-PMIC PWM, inverse PWM
-Asus T100HA  CHT + CRC-PMIC PWM
-Terra Pad 1061  BYT + LPSS PWM
-Trekstor Twin 10.1 BYT + LPSS PWM
-Asus T101HA  CHT + CRC-PMIC PWM
-GPD Pocket  CHT + CRC-PMIC PWM

Changelog:

Changes in v2:
- Fix coverletter subject
- Drop accidentally included debugging patch
- "[PATCH v3 02/15] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (
  - Move #define LPSS_SAVE_CTX_ONCE define to group it with LPSS_SAVE_CTX

Changes in v3:
- "[PATCH v3 04/15] pwm: lpss: Add range limit check for the base_unit register value"
  - Use base_unit_range - 1 as maximum value for the clamp()
- "[PATCH v3 05/15] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume"
  - This replaces the "pwm: lpss: Set SW_UPDATE bit when enabling the PWM"
    patch from previous versions of this patch-set, which really was a hack
    working around the resume issue which this patch fixes properly.
- PATCH v3 6 - 11 pwm-crc changes:
  - Various small changes resulting from the reviews by Andy and Uwe,
    including some refactoring of the patches to reduce the amount of churn
    in the patch-set

Changes in v4:
- "[PATCH v4 06/16] pwm: lpss: Correct get_state result for base_unit == 0"
  - This is a new patch in v4 of this patchset
- "[PATCH v4 12/16] pwm: crc: Implement get_state() method"
  - Use DIV_ROUND_UP when calculating the period and duty_cycle values
- "[PATCH v4 16/16] drm/i915: panel: Use atomic PWM API for devs with an external PWM controller"
  - Add a note to the commit message about the changes in pwm_disable_backlight()
  - Use the pwm_set/get_relative_duty_cycle() helpers

Regards,

Hans

Comments

Hans de Goede July 9, 2020, 7:09 a.m. UTC | #1
Hi,

On 09-07-2020 05:33, Patchwork wrote:
> == Series Details ==
> 
> Series: acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API (rev2)
> URL   : https://patchwork.freedesktop.org/series/78657/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_8717_full -> Patchwork_18116_full
> ====================================================
> 
> Summary
> -------
> 
>    **FAILURE**
> 
>    Serious unknown changes coming with Patchwork_18116_full absolutely need to be
>    verified manually.
>    
>    If you think the reported changes have nothing to do with the changes
>    introduced in Patchwork_18116_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_18116_full:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>    * igt@i915_module_load@reload-with-fault-injection:
>      - shard-hsw:          [PASS][1] -> [INCOMPLETE][2]
>     [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-hsw6/igt@i915_module_load@reload-with-fault-injection.html
>     [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-hsw7/igt@i915_module_load@reload-with-fault-injection.html

This failure seems unrelated to the changes this series makes, so I plan to ignore
this failure.

Regards,

Hans




> 
>    
> Known issues
> ------------
> 
>    Here are the changes found in Patchwork_18116_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>    * igt@gem_exec_reloc@basic-concurrent0:
>      - shard-glk:          [PASS][3] -> [FAIL][4] ([i915#1930])
>     [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-glk3/igt@gem_exec_reloc@basic-concurrent0.html
>     [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-glk7/igt@gem_exec_reloc@basic-concurrent0.html
> 
>    * igt@gem_exec_whisper@basic-contexts:
>      - shard-glk:          [PASS][5] -> [DMESG-WARN][6] ([i915#118] / [i915#95]) +1 similar issue
>     [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-glk4/igt@gem_exec_whisper@basic-contexts.html
>     [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-glk9/igt@gem_exec_whisper@basic-contexts.html
> 
>    * igt@i915_module_load@reload:
>      - shard-tglb:         [PASS][7] -> [DMESG-WARN][8] ([i915#402]) +1 similar issue
>     [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-tglb2/igt@i915_module_load@reload.html
>     [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-tglb2/igt@i915_module_load@reload.html
> 
>    * igt@kms_big_fb@linear-16bpp-rotate-0:
>      - shard-kbl:          [PASS][9] -> [DMESG-FAIL][10] ([i915#95])
>     [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-kbl1/igt@kms_big_fb@linear-16bpp-rotate-0.html
>     [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-kbl7/igt@kms_big_fb@linear-16bpp-rotate-0.html
>      - shard-apl:          [PASS][11] -> [DMESG-FAIL][12] ([i915#1635] / [i915#95])
>     [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl7/igt@kms_big_fb@linear-16bpp-rotate-0.html
>     [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl1/igt@kms_big_fb@linear-16bpp-rotate-0.html
> 
>    * igt@kms_color@pipe-c-ctm-0-25:
>      - shard-skl:          [PASS][13] -> [DMESG-WARN][14] ([i915#1982]) +7 similar issues
>     [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-skl8/igt@kms_color@pipe-c-ctm-0-25.html
>     [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-skl2/igt@kms_color@pipe-c-ctm-0-25.html
> 
>    * igt@kms_cursor_crc@pipe-b-cursor-128x128-rapid-movement:
>      - shard-kbl:          [PASS][15] -> [DMESG-WARN][16] ([i915#93] / [i915#95]) +1 similar issue
>     [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-kbl6/igt@kms_cursor_crc@pipe-b-cursor-128x128-rapid-movement.html
>     [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-128x128-rapid-movement.html
> 
>    * igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic:
>      - shard-apl:          [PASS][17] -> [DMESG-WARN][18] ([i915#1635] / [i915#95]) +23 similar issues
>     [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl3/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
>     [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl1/igt@kms_cursor_legacy@long-nonblocking-modeset-vs-cursor-atomic.html
> 
>    * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-vga1-hdmi-a1:
>      - shard-hsw:          [PASS][19] -> [FAIL][20] ([i915#79])
>     [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-hsw4/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-vga1-hdmi-a1.html
>     [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-hsw4/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@bc-vga1-hdmi-a1.html
> 
>    * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
>      - shard-kbl:          [PASS][21] -> [DMESG-WARN][22] ([i915#180]) +6 similar issues
>     [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
>     [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-kbl4/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
> 
>    * igt@kms_flip@plain-flip-fb-recreate@c-edp1:
>      - shard-skl:          [PASS][23] -> [FAIL][24] ([i915#2122])
>     [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-skl2/igt@kms_flip@plain-flip-fb-recreate@c-edp1.html
>     [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-skl4/igt@kms_flip@plain-flip-fb-recreate@c-edp1.html
> 
>    * igt@kms_frontbuffer_tracking@fbcpsr-shrfb-scaledprimary:
>      - shard-tglb:         [PASS][25] -> [DMESG-WARN][26] ([i915#1982]) +1 similar issue
>     [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-shrfb-scaledprimary.html
>     [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-shrfb-scaledprimary.html
> 
>    * igt@kms_psr2_su@page_flip:
>      - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109642] / [fdo#111068])
>     [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-iclb2/igt@kms_psr2_su@page_flip.html
>     [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-iclb5/igt@kms_psr2_su@page_flip.html
> 
>    * igt@kms_psr@psr2_suspend:
>      - shard-iclb:         [PASS][29] -> [SKIP][30] ([fdo#109441]) +1 similar issue
>     [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-iclb2/igt@kms_psr@psr2_suspend.html
>     [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-iclb6/igt@kms_psr@psr2_suspend.html
> 
>    * igt@perf@blocking-parameterized:
>      - shard-iclb:         [PASS][31] -> [FAIL][32] ([i915#1542])
>     [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-iclb1/igt@perf@blocking-parameterized.html
>     [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-iclb3/igt@perf@blocking-parameterized.html
> 
>    * igt@perf_pmu@semaphore-busy@rcs0:
>      - shard-kbl:          [PASS][33] -> [FAIL][34] ([i915#1820])
>     [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-kbl4/igt@perf_pmu@semaphore-busy@rcs0.html
>     [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-kbl1/igt@perf_pmu@semaphore-busy@rcs0.html
> 
>    
> #### Possible fixes ####
> 
>    * igt@gem_exec_whisper@basic-contexts-forked:
>      - shard-glk:          [DMESG-WARN][35] ([i915#118] / [i915#95]) -> [PASS][36]
>     [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-glk6/igt@gem_exec_whisper@basic-contexts-forked.html
>     [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-glk6/igt@gem_exec_whisper@basic-contexts-forked.html
> 
>    * igt@gen9_exec_parse@allowed-all:
>      - shard-apl:          [DMESG-WARN][37] ([i915#1436] / [i915#716]) -> [PASS][38]
>     [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl6/igt@gen9_exec_parse@allowed-all.html
>     [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl2/igt@gen9_exec_parse@allowed-all.html
> 
>    * igt@i915_module_load@reload:
>      - shard-apl:          [DMESG-WARN][39] ([i915#1982]) -> [PASS][40]
>     [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl1/igt@i915_module_load@reload.html
>     [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl7/igt@i915_module_load@reload.html
> 
>    * igt@kms_big_fb@linear-64bpp-rotate-180:
>      - shard-glk:          [DMESG-FAIL][41] ([i915#118] / [i915#95]) -> [PASS][42] +1 similar issue
>     [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-glk8/igt@kms_big_fb@linear-64bpp-rotate-180.html
>     [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-glk3/igt@kms_big_fb@linear-64bpp-rotate-180.html
> 
>    * igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding:
>      - shard-tglb:         [DMESG-WARN][43] ([i915#402]) -> [PASS][44]
>     [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-tglb2/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html
>     [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-tglb7/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html
> 
>    * igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen:
>      - shard-apl:          [DMESG-WARN][45] ([i915#1635] / [i915#95]) -> [PASS][46] +19 similar issues
>     [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl6/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html
>     [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl3/igt@kms_cursor_crc@pipe-c-cursor-256x85-offscreen.html
> 
>    * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
>      - shard-skl:          [DMESG-WARN][47] ([i915#1982]) -> [PASS][48] +6 similar issues
>     [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-skl7/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
>     [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-skl7/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
> 
>    * igt@kms_flip@flip-vs-suspend@c-dp1:
>      - shard-kbl:          [DMESG-WARN][49] ([i915#180]) -> [PASS][50] +1 similar issue
>     [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html
>     [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html
> 
>    * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
>      - shard-skl:          [FAIL][51] ([i915#49]) -> [PASS][52]
>     [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu.html
>     [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-skl6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu.html
> 
>    * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite:
>      - shard-tglb:         [DMESG-WARN][53] ([i915#1982]) -> [PASS][54] +1 similar issue
>     [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html
>     [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html
> 
>    * igt@kms_hdr@bpc-switch:
>      - shard-skl:          [FAIL][55] ([i915#1188]) -> [PASS][56]
>     [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-skl1/igt@kms_hdr@bpc-switch.html
>     [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-skl5/igt@kms_hdr@bpc-switch.html
> 
>    * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
>      - shard-skl:          [FAIL][57] ([fdo#108145] / [i915#265]) -> [PASS][58] +2 similar issues
>     [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
>     [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
> 
>    * igt@kms_psr@psr2_sprite_render:
>      - shard-iclb:         [SKIP][59] ([fdo#109441]) -> [PASS][60] +2 similar issues
>     [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-iclb5/igt@kms_psr@psr2_sprite_render.html
>     [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-iclb2/igt@kms_psr@psr2_sprite_render.html
> 
>    
> #### Warnings ####
> 
>    * igt@kms_chamelium@dp-crc-fast:
>      - shard-apl:          [SKIP][61] ([fdo#109271] / [fdo#111827] / [i915#1635]) -> [SKIP][62] ([fdo#109271] / [fdo#111827])
>     [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl4/igt@kms_chamelium@dp-crc-fast.html
>     [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl3/igt@kms_chamelium@dp-crc-fast.html
> 
>    * igt@kms_chamelium@hdmi-crc-fast:
>      - shard-apl:          [SKIP][63] ([fdo#109271] / [fdo#111827]) -> [SKIP][64] ([fdo#109271] / [fdo#111827] / [i915#1635])
>     [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl8/igt@kms_chamelium@hdmi-crc-fast.html
>     [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl7/igt@kms_chamelium@hdmi-crc-fast.html
> 
>    * igt@kms_cursor_legacy@cursora-vs-flipb-varying-size:
>      - shard-apl:          [SKIP][65] ([fdo#109271]) -> [SKIP][66] ([fdo#109271] / [i915#1635]) +8 similar issues
>     [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl2/igt@kms_cursor_legacy@cursora-vs-flipb-varying-size.html
>     [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl6/igt@kms_cursor_legacy@cursora-vs-flipb-varying-size.html
> 
>    * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
>      - shard-apl:          [FAIL][67] ([i915#49]) -> [DMESG-FAIL][68] ([i915#1635] / [i915#49] / [i915#95])
>     [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
>     [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
> 
>    * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
>      - shard-apl:          [DMESG-FAIL][69] ([fdo#108145] / [i915#1635] / [i915#95]) -> [FAIL][70] ([fdo#108145] / [i915#265])
>     [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl3/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
>     [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl4/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
> 
>    * igt@kms_plane_alpha_blend@pipe-d-coverage-vs-premult-vs-constant:
>      - shard-apl:          [SKIP][71] ([fdo#109271] / [i915#1635]) -> [SKIP][72] ([fdo#109271]) +6 similar issues
>     [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl7/igt@kms_plane_alpha_blend@pipe-d-coverage-vs-premult-vs-constant.html
>     [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl8/igt@kms_plane_alpha_blend@pipe-d-coverage-vs-premult-vs-constant.html
> 
>    * igt@kms_vblank@pipe-b-ts-continuation-suspend:
>      - shard-kbl:          [DMESG-WARN][73] ([i915#93] / [i915#95]) -> [DMESG-WARN][74] ([i915#180] / [i915#93] / [i915#95])
>     [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-kbl3/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
>     [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-kbl3/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
> 
>    * igt@runner@aborted:
>      - shard-apl:          ([FAIL][75], [FAIL][76], [FAIL][77]) ([fdo#109271] / [i915#1610] / [i915#1635] / [i915#2110] / [i915#716]) -> [FAIL][78] ([i915#1635] / [i915#2110])
>     [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl6/igt@runner@aborted.html
>     [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl2/igt@runner@aborted.html
>     [77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8717/shard-apl6/igt@runner@aborted.html
>     [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18116/shard-apl8/igt@runner@aborted.html
> 
>    
>    [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>    [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>    [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>    [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
>    [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
>    [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
>    [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
>    [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
>    [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
>    [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
>    [i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
>    [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
>    [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
>    [i915#1820]: https://gitlab.freedesktop.org/drm/intel/issues/1820
>    [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
>    [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
>    [i915#2110]: https://gitlab.freedesktop.org/drm/intel/issues/2110
>    [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
>    [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
>    [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
>    [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
>    [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
>    [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
>    [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
>    [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
> 
> 
> Participating hosts (10 -> 10)
> ------------------------------
> 
>    No changes in participating hosts
> 
> 
> Build changes
> -------------
> 
>    * Linux: CI_DRM_8717 -> Patchwork_18116
> 
>    CI-20190529: 20190529
>    CI_DRM_8717: 6e5ac4c72af4e86138f04cc2dd089b069bad873f @ git://anongit.freedesktop.org/gfx-ci/linux
>    IGT_5728: 6988eebf78e9ce9746b8c2b7d21cb4174d6623a9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>    Patchwork_18116: f5cb7cd44c63bc13aa41fa6d9b9776a5318bb601 @ 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_18116/index.html
>
Sam Ravnborg July 9, 2020, 2:14 p.m. UTC | #2
Hi Hans.

On Wed, Jul 08, 2020 at 11:14:16PM +0200, Hans de Goede wrote:
> Hi All,
> 
> Here is v4 of my patch series converting the i915 driver's code for
> controlling the panel's backlight with an external PWM controller to
> use the atomic PWM API. See below for the changelog.

Why is it that i915 cannot use the pwm_bl driver for backlight?
I have not studied the code - just wondering.

	Sam

> 
> Initially the plan was for this series to consist of 2 parts:
> 1. convert the pwm-crc driver to support the atomic PWM API and
> 2. convert the i915 driver's PWM code to use the atomic PWM API.
> 
> But during testing I've found a number of bugs in the pwm-lpss and I
> found that the acpi_lpss code needs some special handling because of
> some ugliness found in most Cherry Trail DSDTs.
> 
> So now this series has grown somewhat large and consists of 4 parts:
> 
> 1. acpi_lpss fixes workarounds for Cherry Trail DSTD nastiness
> 2. various fixes to the pwm-lpss driver
> 3. convert the pwm-crc driver to support the atomic PWM API and
> 4. convert the i915 driver's PWM code to use the atomic PWM API
> 
> The involved acpi_lpss and pwm drivers do not see a whole lot of churn,
> so the plan is to merge this all through drm-intel-next-queued (dinq)
> once all the patches are reviewed / have acks.
> 
> In v4 the ACPI patches have been Acked by Rafael and the i915 patches
> have been acked by Jani. So that just leaves the PWM patches.
> 
> Uwe can I get your ok / ack for merging this through the dinq branch
> once you have acked al the PWM patches ?
> 
> This series has been tested (and re-tested after adding various bug-fixes)
> extensively. It has been tested on the following devices:
> 
> -Asus T100TA  BYT + CRC-PMIC PWM
> -Toshiba WT8-A  BYT + CRC-PMIC PWM
> -Thundersoft TS178 BYT + CRC-PMIC PWM, inverse PWM
> -Asus T100HA  CHT + CRC-PMIC PWM
> -Terra Pad 1061  BYT + LPSS PWM
> -Trekstor Twin 10.1 BYT + LPSS PWM
> -Asus T101HA  CHT + CRC-PMIC PWM
> -GPD Pocket  CHT + CRC-PMIC PWM
> 
> Changelog:
> 
> Changes in v2:
> - Fix coverletter subject
> - Drop accidentally included debugging patch
> - "[PATCH v3 02/15] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (
>   - Move #define LPSS_SAVE_CTX_ONCE define to group it with LPSS_SAVE_CTX
> 
> Changes in v3:
> - "[PATCH v3 04/15] pwm: lpss: Add range limit check for the base_unit register value"
>   - Use base_unit_range - 1 as maximum value for the clamp()
> - "[PATCH v3 05/15] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume"
>   - This replaces the "pwm: lpss: Set SW_UPDATE bit when enabling the PWM"
>     patch from previous versions of this patch-set, which really was a hack
>     working around the resume issue which this patch fixes properly.
> - PATCH v3 6 - 11 pwm-crc changes:
>   - Various small changes resulting from the reviews by Andy and Uwe,
>     including some refactoring of the patches to reduce the amount of churn
>     in the patch-set
> 
> Changes in v4:
> - "[PATCH v4 06/16] pwm: lpss: Correct get_state result for base_unit == 0"
>   - This is a new patch in v4 of this patchset
> - "[PATCH v4 12/16] pwm: crc: Implement get_state() method"
>   - Use DIV_ROUND_UP when calculating the period and duty_cycle values
> - "[PATCH v4 16/16] drm/i915: panel: Use atomic PWM API for devs with an external PWM controller"
>   - Add a note to the commit message about the changes in pwm_disable_backlight()
>   - Use the pwm_set/get_relative_duty_cycle() helpers
> 
> Regards,
> 
> Hans
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hans de Goede July 9, 2020, 2:40 p.m. UTC | #3
Hi,

On 7/9/20 4:14 PM, Sam Ravnborg wrote:
> Hi Hans.
> 
> On Wed, Jul 08, 2020 at 11:14:16PM +0200, Hans de Goede wrote:
>> Hi All,
>>
>> Here is v4 of my patch series converting the i915 driver's code for
>> controlling the panel's backlight with an external PWM controller to
>> use the atomic PWM API. See below for the changelog.
> 
> Why is it that i915 cannot use the pwm_bl driver for backlight?
> I have not studied the code - just wondering.

The intel_panel.c code deals with 7 different types of PWM controllers
which are built into the GPU + support for external PWM controllers
through the kernel's PWM subsystem.

pwm_bl will work for the external PWM controller case, but not for
the others. On top of that the intel_panel code integrates which
the video BIOS, getting things like frequency, minimum value
and if the range is inverted (0% duty == backlight brightness max).
I'm not even sure if pwm_bl supports all of this, but even if it
does the intel_panel code handles this in a unified manner for
all supported PWM controllers, including the ones which are
an integral part of the GPU.

Regards,

Hans



>> Initially the plan was for this series to consist of 2 parts:
>> 1. convert the pwm-crc driver to support the atomic PWM API and
>> 2. convert the i915 driver's PWM code to use the atomic PWM API.
>>
>> But during testing I've found a number of bugs in the pwm-lpss and I
>> found that the acpi_lpss code needs some special handling because of
>> some ugliness found in most Cherry Trail DSDTs.
>>
>> So now this series has grown somewhat large and consists of 4 parts:
>>
>> 1. acpi_lpss fixes workarounds for Cherry Trail DSTD nastiness
>> 2. various fixes to the pwm-lpss driver
>> 3. convert the pwm-crc driver to support the atomic PWM API and
>> 4. convert the i915 driver's PWM code to use the atomic PWM API
>>
>> The involved acpi_lpss and pwm drivers do not see a whole lot of churn,
>> so the plan is to merge this all through drm-intel-next-queued (dinq)
>> once all the patches are reviewed / have acks.
>>
>> In v4 the ACPI patches have been Acked by Rafael and the i915 patches
>> have been acked by Jani. So that just leaves the PWM patches.
>>
>> Uwe can I get your ok / ack for merging this through the dinq branch
>> once you have acked al the PWM patches ?
>>
>> This series has been tested (and re-tested after adding various bug-fixes)
>> extensively. It has been tested on the following devices:
>>
>> -Asus T100TA  BYT + CRC-PMIC PWM
>> -Toshiba WT8-A  BYT + CRC-PMIC PWM
>> -Thundersoft TS178 BYT + CRC-PMIC PWM, inverse PWM
>> -Asus T100HA  CHT + CRC-PMIC PWM
>> -Terra Pad 1061  BYT + LPSS PWM
>> -Trekstor Twin 10.1 BYT + LPSS PWM
>> -Asus T101HA  CHT + CRC-PMIC PWM
>> -GPD Pocket  CHT + CRC-PMIC PWM
>>
>> Changelog:
>>
>> Changes in v2:
>> - Fix coverletter subject
>> - Drop accidentally included debugging patch
>> - "[PATCH v3 02/15] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (
>>    - Move #define LPSS_SAVE_CTX_ONCE define to group it with LPSS_SAVE_CTX
>>
>> Changes in v3:
>> - "[PATCH v3 04/15] pwm: lpss: Add range limit check for the base_unit register value"
>>    - Use base_unit_range - 1 as maximum value for the clamp()
>> - "[PATCH v3 05/15] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume"
>>    - This replaces the "pwm: lpss: Set SW_UPDATE bit when enabling the PWM"
>>      patch from previous versions of this patch-set, which really was a hack
>>      working around the resume issue which this patch fixes properly.
>> - PATCH v3 6 - 11 pwm-crc changes:
>>    - Various small changes resulting from the reviews by Andy and Uwe,
>>      including some refactoring of the patches to reduce the amount of churn
>>      in the patch-set
>>
>> Changes in v4:
>> - "[PATCH v4 06/16] pwm: lpss: Correct get_state result for base_unit == 0"
>>    - This is a new patch in v4 of this patchset
>> - "[PATCH v4 12/16] pwm: crc: Implement get_state() method"
>>    - Use DIV_ROUND_UP when calculating the period and duty_cycle values
>> - "[PATCH v4 16/16] drm/i915: panel: Use atomic PWM API for devs with an external PWM controller"
>>    - Add a note to the commit message about the changes in pwm_disable_backlight()
>>    - Use the pwm_set/get_relative_duty_cycle() helpers
>>
>> Regards,
>>
>> Hans
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Sam Ravnborg July 9, 2020, 3:23 p.m. UTC | #4
Hi,
On Thu, Jul 09, 2020 at 04:40:56PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 7/9/20 4:14 PM, Sam Ravnborg wrote:
> > Hi Hans.
> > 
> > On Wed, Jul 08, 2020 at 11:14:16PM +0200, Hans de Goede wrote:
> > > Hi All,
> > > 
> > > Here is v4 of my patch series converting the i915 driver's code for
> > > controlling the panel's backlight with an external PWM controller to
> > > use the atomic PWM API. See below for the changelog.
> > 
> > Why is it that i915 cannot use the pwm_bl driver for backlight?
> > I have not studied the code - just wondering.
> 
> The intel_panel.c code deals with 7 different types of PWM controllers
> which are built into the GPU + support for external PWM controllers
> through the kernel's PWM subsystem.
> 
> pwm_bl will work for the external PWM controller case, but not for
> the others. On top of that the intel_panel code integrates which
> the video BIOS, getting things like frequency, minimum value
> and if the range is inverted (0% duty == backlight brightness max).
> I'm not even sure if pwm_bl supports all of this, but even if it
> does the intel_panel code handles this in a unified manner for
> all supported PWM controllers, including the ones which are
> an integral part of the GPU.

Thanks for the explanation.
This is a more complicated world than the usual embedded case with a
single pwm, no BIOS etc. So it makes sense.

	Sam

> 
> Regards,
> 
> Hans
> 
> 
> 
> > > Initially the plan was for this series to consist of 2 parts:
> > > 1. convert the pwm-crc driver to support the atomic PWM API and
> > > 2. convert the i915 driver's PWM code to use the atomic PWM API.
> > > 
> > > But during testing I've found a number of bugs in the pwm-lpss and I
> > > found that the acpi_lpss code needs some special handling because of
> > > some ugliness found in most Cherry Trail DSDTs.
> > > 
> > > So now this series has grown somewhat large and consists of 4 parts:
> > > 
> > > 1. acpi_lpss fixes workarounds for Cherry Trail DSTD nastiness
> > > 2. various fixes to the pwm-lpss driver
> > > 3. convert the pwm-crc driver to support the atomic PWM API and
> > > 4. convert the i915 driver's PWM code to use the atomic PWM API
> > > 
> > > The involved acpi_lpss and pwm drivers do not see a whole lot of churn,
> > > so the plan is to merge this all through drm-intel-next-queued (dinq)
> > > once all the patches are reviewed / have acks.
> > > 
> > > In v4 the ACPI patches have been Acked by Rafael and the i915 patches
> > > have been acked by Jani. So that just leaves the PWM patches.
> > > 
> > > Uwe can I get your ok / ack for merging this through the dinq branch
> > > once you have acked al the PWM patches ?
> > > 
> > > This series has been tested (and re-tested after adding various bug-fixes)
> > > extensively. It has been tested on the following devices:
> > > 
> > > -Asus T100TA  BYT + CRC-PMIC PWM
> > > -Toshiba WT8-A  BYT + CRC-PMIC PWM
> > > -Thundersoft TS178 BYT + CRC-PMIC PWM, inverse PWM
> > > -Asus T100HA  CHT + CRC-PMIC PWM
> > > -Terra Pad 1061  BYT + LPSS PWM
> > > -Trekstor Twin 10.1 BYT + LPSS PWM
> > > -Asus T101HA  CHT + CRC-PMIC PWM
> > > -GPD Pocket  CHT + CRC-PMIC PWM
> > > 
> > > Changelog:
> > > 
> > > Changes in v2:
> > > - Fix coverletter subject
> > > - Drop accidentally included debugging patch
> > > - "[PATCH v3 02/15] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (
> > >    - Move #define LPSS_SAVE_CTX_ONCE define to group it with LPSS_SAVE_CTX
> > > 
> > > Changes in v3:
> > > - "[PATCH v3 04/15] pwm: lpss: Add range limit check for the base_unit register value"
> > >    - Use base_unit_range - 1 as maximum value for the clamp()
> > > - "[PATCH v3 05/15] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume"
> > >    - This replaces the "pwm: lpss: Set SW_UPDATE bit when enabling the PWM"
> > >      patch from previous versions of this patch-set, which really was a hack
> > >      working around the resume issue which this patch fixes properly.
> > > - PATCH v3 6 - 11 pwm-crc changes:
> > >    - Various small changes resulting from the reviews by Andy and Uwe,
> > >      including some refactoring of the patches to reduce the amount of churn
> > >      in the patch-set
> > > 
> > > Changes in v4:
> > > - "[PATCH v4 06/16] pwm: lpss: Correct get_state result for base_unit == 0"
> > >    - This is a new patch in v4 of this patchset
> > > - "[PATCH v4 12/16] pwm: crc: Implement get_state() method"
> > >    - Use DIV_ROUND_UP when calculating the period and duty_cycle values
> > > - "[PATCH v4 16/16] drm/i915: panel: Use atomic PWM API for devs with an external PWM controller"
> > >    - Add a note to the commit message about the changes in pwm_disable_backlight()
> > >    - Use the pwm_set/get_relative_duty_cycle() helpers
> > > 
> > > Regards,
> > > 
> > > Hans
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
Uwe Kleine-König July 11, 2020, 6:19 a.m. UTC | #5
Hi Hans,

On Thu, Jul 09, 2020 at 04:40:56PM +0200, Hans de Goede wrote:
> On 7/9/20 4:14 PM, Sam Ravnborg wrote:
> > On Wed, Jul 08, 2020 at 11:14:16PM +0200, Hans de Goede wrote:
> > > Here is v4 of my patch series converting the i915 driver's code for
> > > controlling the panel's backlight with an external PWM controller to
> > > use the atomic PWM API. See below for the changelog.
> > 
> > Why is it that i915 cannot use the pwm_bl driver for backlight?
> > I have not studied the code - just wondering.
> 
> The intel_panel.c code deals with 7 different types of PWM controllers
> which are built into the GPU + support for external PWM controllers
> through the kernel's PWM subsystem.
> 
> pwm_bl will work for the external PWM controller case, but not for
> the others. On top of that the intel_panel code integrates which
> the video BIOS, getting things like frequency, minimum value
> and if the range is inverted (0% duty == backlight brightness max).
> I'm not even sure if pwm_bl supports all of this, but even if it
> does the intel_panel code handles this in a unified manner for
> all supported PWM controllers, including the ones which are
> an integral part of the GPU.

pwm_bl handles inverted PWM just fine. I'm unsure what "integrates which
the video BIOS" means, but I don't see how "handling 7 different types
of PWM controllers explicitly and others using the PWM API" can be seen
as "unified manner" compared to "provide a pwm driver for whatever might
be in the GPU and then use generic code (PWM API, pwm_bl) to drive it".

Maybe I'm not understanding some involved complexity here?

Best regards
Uwe
Hans de Goede July 11, 2020, 1:46 p.m. UTC | #6
Hi,

On 7/11/20 8:19 AM, Uwe Kleine-König wrote:
> Hi Hans,
> 
> On Thu, Jul 09, 2020 at 04:40:56PM +0200, Hans de Goede wrote:
>> On 7/9/20 4:14 PM, Sam Ravnborg wrote:
>>> On Wed, Jul 08, 2020 at 11:14:16PM +0200, Hans de Goede wrote:
>>>> Here is v4 of my patch series converting the i915 driver's code for
>>>> controlling the panel's backlight with an external PWM controller to
>>>> use the atomic PWM API. See below for the changelog.
>>>
>>> Why is it that i915 cannot use the pwm_bl driver for backlight?
>>> I have not studied the code - just wondering.
>>
>> The intel_panel.c code deals with 7 different types of PWM controllers
>> which are built into the GPU + support for external PWM controllers
>> through the kernel's PWM subsystem.
>>
>> pwm_bl will work for the external PWM controller case, but not for
>> the others. On top of that the intel_panel code integrates which
>> the video BIOS, getting things like frequency, minimum value
>> and if the range is inverted (0% duty == backlight brightness max).
>> I'm not even sure if pwm_bl supports all of this, but even if it
>> does the intel_panel code handles this in a unified manner for
>> all supported PWM controllers, including the ones which are
>> an integral part of the GPU.
> 
> pwm_bl handles inverted PWM just fine. I'm unsure what "integrates which
> the video BIOS" means,

Integrating with the video BIOS means reading the VBT (Video BIOS Tables)
and extracting info about which PWM controller to use, what frequency
to program the output at, minimum allowed duty-cycle and if the scale
is inverted.

> but I don't see how "handling 7 different types
> of PWM controllers explicitly and others using the PWM API" can be seen
> as "unified manner" compared to "provide a pwm driver for whatever might
> be in the GPU and then use generic code (PWM API, pwm_bl) to drive it".

Part of this is historical, the main x86 GPU drivers have always treated
backlight control as integral part of the display pipeline and in some
cases it really is, e.g. for eDP panels in some cases the backlight
is controlled through the DP aux channel, there is no PWM controller
(visible to the kernel involved). So the intel_panel.c code really
is a backlight-control de-multiplexer, picking the right "plugin"
to control the backlight, which may also be the eDP backlight control
code. Using a PWM controller supported by the PWM-core/class is just
one of the many supported "plugins".

Also the GPU currently is treated as a single device, not as a MFD
device, so we cannot have an isolated PWM driver. We could have code
inside the GPU driver which exports a PWM-controller to the PWM-core,
to then get a reference to the exported PWM-controller but that would
be very roundabout.

The devices which are using an external PWM controller are actually
the exception here, 99.9% of all devices use the GPU integrated PWM
controller.

Anyways changing over the other PWM-like controllers support by
the intel-panel code falls way outside of the scope of this
patch-set.

Regards,

Hans