[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/4] kms_content_protection: Fix log bug on 32-bit platforms.
== Series Details == Series: series starting with [1/4] kms_content_protection: Fix log bug on 32-bit platforms. URL : https://patchwork.freedesktop.org/series/52505/ State : failure == Summary == = CI Bug Log - changes from IGT_4714_full -> IGTPW_2062_full = == Summary - FAILURE == Serious unknown changes coming with IGTPW_2062_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_2062_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/52505/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in IGTPW_2062_full: === IGT changes === Possible regressions igt@gem_exec_reuse@baggage: shard-apl: PASS -> DMESG-WARN Warnings igt@kms_cursor_legacy@2x-flip-vs-cursor-legacy: shard-hsw: PASS -> SKIP == Known issues == Here are the changes found in IGTPW_2062_full that come from known issues: === IGT changes === Issues hit igt@gem_ppgtt@blt-vs-render-ctxn: shard-kbl: PASS -> INCOMPLETE (fdo#106887, fdo#106023, fdo#103665) igt@gem_workarounds@suspend-resume: shard-kbl: PASS -> DMESG-WARN (fdo#103313) igt@kms_atomic@plane_overlay_legacy: shard-apl: PASS -> INCOMPLETE (fdo#103927) igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c: shard-kbl: NOTRUN -> DMESG-WARN (fdo#107956) +1 igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a: shard-apl: PASS -> DMESG-WARN (fdo#107956) igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c: shard-glk: NOTRUN -> DMESG-WARN (fdo#107956) +1 igt@kms_ccs@pipe-b-crc-sprite-planes-basic: shard-glk: PASS -> FAIL (fdo#108145) shard-kbl: NOTRUN -> FAIL (fdo#107725, fdo#108145) igt@kms_color@pipe-b-degamma: shard-apl: PASS -> FAIL (fdo#104782) igt@kms_color@pipe-c-ctm-blue-to-red: shard-kbl: PASS -> DMESG-WARN (fdo#103313, fdo#105345) igt@kms_cursor_crc@cursor-256x256-sliding: shard-glk: PASS -> FAIL (fdo#103232) shard-kbl: NOTRUN -> FAIL (fdo#103232) igt@kms_cursor_crc@cursor-256x85-random: shard-apl: PASS -> FAIL (fdo#103232) +5 igt@kms_cursor_crc@cursor-64x64-dpms: shard-kbl: PASS -> FAIL (fdo#103232) +4 igt@kms_flip@2x-modeset-vs-vblank-race-interruptible: shard-glk: PASS -> FAIL (fdo#103060) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-cpu: shard-glk: PASS -> FAIL (fdo#103167) shard-apl: PASS -> FAIL (fdo#103167) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite: shard-glk: NOTRUN -> FAIL (fdo#103167) +2 igt@kms_plane@plane-position-covered-pipe-a-planes: shard-glk: NOTRUN -> FAIL (fdo#103166) +2 shard-kbl: PASS -> FAIL (fdo#103166) +2 igt@kms_plane@plane-position-covered-pipe-b-planes: shard-glk: PASS -> FAIL (fdo#103166) +1 igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb: shard-glk: NOTRUN -> FAIL (fdo#108145) +8 igt@kms_plane_multiple@atomic-pipe-a-tiling-x: shard-apl: PASS -> FAIL (fdo#103166) +5 igt@kms_properties@connector-properties-atomic: shard-glk: NOTRUN -> FAIL (fdo#108642) shard-hsw: NOTRUN -> FAIL (fdo#108642) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) Possible fixes igt@drv_suspend@forcewake: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@drv_suspend@shrink: shard-snb: INCOMPLETE (fdo#105411, fdo#106886) -> PASS igt@gem_ppgtt@blt-vs-render-ctx0: shard-kbl: INCOMPLETE (fdo#106887, fdo#106023, fdo#103665) -> PASS igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a: shard-hsw: DMESG-WARN (fdo#107956) -> PASS igt@kms_cursor_crc@cursor-128x42-sliding: shard-kbl: FAIL (fdo#103232) -> PASS +3 igt@kms_cursor_crc@cursor-256x85-random: shard-glk: FAIL (fdo#103232) -> PASS +1 igt@kms_cursor_crc@cursor-64x21-sliding: shard-apl: FAIL (fdo#103232) -> PASS +3 igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite: shard-apl: FAIL (fdo#103167) -> PASS igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff: shard-glk: FAIL (fdo#103167) -> PASS +4 igt@kms_plane_multiple@atomic-pipe-b-tiling-y: shard-apl: FAIL (fdo#103166) -> PASS igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
[Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: Program SKL+ watermarks/ddb more carefully (rev7)
== Series Details == Series: drm/i915: Program SKL+ watermarks/ddb more carefully (rev7) URL : https://patchwork.freedesktop.org/series/51878/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_5140_full -> Patchwork_10827_full = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10827_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10827_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_10827_full: === IGT changes === Possible regressions igt@prime_vgem@basic-fence-flip: shard-apl: PASS -> DMESG-WARN shard-kbl: PASS -> DMESG-WARN Warnings igt@pm_rc6_residency@rc6-accuracy: shard-snb: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_10827_full that come from known issues: === IGT changes === Issues hit igt@gem_cpu_reloc@full: shard-skl: NOTRUN -> INCOMPLETE (fdo#108073) igt@gem_ctx_isolation@vecs0-s3: shard-skl: PASS -> INCOMPLETE (fdo#107773, fdo#104108) igt@kms_busy@extended-pageflip-hang-newfb-render-a: shard-apl: PASS -> DMESG-WARN (fdo#107956) igt@kms_cursor_crc@cursor-256x256-suspend: shard-apl: PASS -> FAIL (fdo#103232, fdo#103191) igt@kms_fbcon_fbt@psr-suspend: shard-skl: NOTRUN -> FAIL (fdo#107882) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite: shard-apl: PASS -> FAIL (fdo#103167) +1 igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move: shard-glk: PASS -> FAIL (fdo#103167) +1 igt@kms_frontbuffer_tracking@fbc-stridechange: shard-skl: NOTRUN -> FAIL (fdo#105683) igt@kms_plane@pixel-format-pipe-c-planes: shard-glk: PASS -> FAIL (fdo#103166) igt@kms_plane@plane-position-covered-pipe-b-planes: shard-apl: PASS -> FAIL (fdo#103166) igt@kms_plane_alpha_blend@pipe-a-alpha-7efc: shard-skl: NOTRUN -> FAIL (fdo#107815, fdo#108145) igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb: shard-apl: NOTRUN -> FAIL (fdo#108145) igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max: shard-glk: PASS -> FAIL (fdo#108145) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) igt@pm_rpm@gem-execbuf: shard-skl: PASS -> INCOMPLETE (fdo#107807, fdo#107803) igt@pm_rpm@pc8-residency: shard-skl: SKIP -> INCOMPLETE (fdo#107807) Possible fixes igt@drv_suspend@shrink: shard-apl: INCOMPLETE (fdo#103927, fdo#106886) -> PASS igt@kms_cursor_crc@cursor-128x128-sliding: shard-apl: FAIL (fdo#103232) -> PASS +1 igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-skl: FAIL (fdo#105363) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt: shard-apl: FAIL (fdo#103167) -> PASS +1 igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff: shard-glk: FAIL (fdo#103167) -> PASS +1 igt@kms_plane_multiple@atomic-pipe-b-tiling-y: shard-glk: FAIL (fdo#103166) -> PASS igt@pm_rpm@dpms-non-lpsp: shard-skl: INCOMPLETE (fdo#107807) -> SKIP igt@pm_rpm@gem-pread: shard-skl: INCOMPLETE (fdo#107807) -> PASS fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232 fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927 fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105683 https://bugs.freedesktop.org/show_bug.cgi?id=105683 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773 fdo#107803 https://bugs.freedesktop.org/show_bug.cgi?id=107803 fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807 fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815 fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882 fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956 fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073 fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 == Participating hosts (6 -> 6) == No changes in participating hosts == Build
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Disable LP3 watermarks on all SNB machines (rev2)
== Series Details == Series: drm/i915: Disable LP3 watermarks on all SNB machines (rev2) URL : https://patchwork.freedesktop.org/series/52440/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5140_full -> Patchwork_10826_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10826_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10826_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_10826_full: === IGT changes === Warnings igt@kms_lease@lease_get: shard-snb: PASS -> SKIP +1 igt@pm_rc6_residency@rc6-accuracy: shard-kbl: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_10826_full that come from known issues: === IGT changes === Issues hit igt@drm_import_export@import-close-race-flink: shard-skl: PASS -> TIMEOUT (fdo#108667) igt@drv_suspend@shrink: shard-skl: PASS -> INCOMPLETE (fdo#106886) igt@gem_exec_await@wide-contexts: shard-apl: PASS -> INCOMPLETE (fdo#103927) igt@gem_ppgtt@blt-vs-render-ctx0: shard-kbl: PASS -> INCOMPLETE (fdo#106887, fdo#106023, fdo#103665) igt@kms_busy@extended-pageflip-hang-newfb-render-a: shard-apl: PASS -> DMESG-WARN (fdo#107956) igt@kms_chv_cursor_fail@pipe-b-64x64-right-edge: shard-glk: PASS -> DMESG-FAIL (fdo#104671, fdo#106538) igt@kms_cursor_crc@cursor-256x256-onscreen: shard-skl: NOTRUN -> FAIL (fdo#103232) igt@kms_cursor_crc@cursor-256x85-random: shard-apl: PASS -> FAIL (fdo#103232) igt@kms_cursor_crc@cursor-64x64-suspend: shard-apl: PASS -> FAIL (fdo#103232, fdo#103191) igt@kms_cursor_legacy@cursorb-vs-flipb-toggle: shard-glk: PASS -> DMESG-WARN (fdo#106538, fdo#105763) igt@kms_fbcon_fbt@psr-suspend: shard-skl: NOTRUN -> FAIL (fdo#107882) igt@kms_flip@blocking-absolute-wf_vblank-interruptible: shard-hsw: PASS -> DMESG-WARN (fdo#102614) igt@kms_flip@modeset-vs-vblank-race-interruptible: shard-glk: PASS -> FAIL (fdo#103060) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt: shard-glk: PASS -> FAIL (fdo#103167) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render: shard-apl: PASS -> FAIL (fdo#103167) igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-render: shard-glk: PASS -> DMESG-FAIL (fdo#106538) igt@kms_frontbuffer_tracking@fbc-stridechange: shard-skl: NOTRUN -> FAIL (fdo#105683) igt@kms_frontbuffer_tracking@fbcpsr-suspend: shard-skl: PASS -> INCOMPLETE (fdo#104108, fdo#106978) igt@kms_plane_alpha_blend@pipe-a-alpha-7efc: shard-skl: NOTRUN -> FAIL (fdo#107815, fdo#108145) igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb: shard-apl: NOTRUN -> FAIL (fdo#108145) igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max: shard-glk: PASS -> FAIL (fdo#108145) igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: shard-skl: NOTRUN -> FAIL (fdo#107815) igt@kms_plane_multiple@atomic-pipe-a-tiling-x: shard-apl: PASS -> FAIL (fdo#103166) +2 igt@kms_plane_multiple@atomic-pipe-b-tiling-y: shard-apl: NOTRUN -> FAIL (fdo#103166) igt@kms_setmode@basic: shard-apl: PASS -> FAIL (fdo#99912) igt@pm_rpm@debugfs-forcewake-user: shard-skl: PASS -> INCOMPLETE (fdo#107807) Possible fixes igt@drv_suspend@forcewake: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@drv_suspend@shrink: shard-apl: INCOMPLETE (fdo#103927, fdo#106886) -> PASS igt@kms_cursor_crc@cursor-128x128-sliding: shard-apl: FAIL (fdo#103232) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff: shard-apl: FAIL (fdo#103167) -> PASS +1 igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff: shard-glk: FAIL (fdo#103167) -> PASS +1 igt@kms_plane_multiple@atomic-pipe-b-tiling-y: shard-glk: FAIL (fdo#103166) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS igt@pm_rpm@dpms-non-lpsp: shard-skl: INCOMPLETE (fdo#107807) -> SKIP igt@pm_rpm@gem-pread: shard-skl: INCOMPLETE (fdo#107807) -> PASS fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 fdo#103166
Re: [Intel-gfx] [PATCH v2 2/3] drm: Add CRTC background color property (v2)
On Wed, Nov 14, 2018 at 11:17:25AM -0500, Sean Paul wrote: > On Tue, Nov 13, 2018 at 03:21:48PM -0800, Matt Roper wrote: > > Some display controllers can be programmed to present non-black colors > > for pixels not covered by any plane (or pixels covered by the > > transparent regions of higher planes). Compositors that want a UI with > > a solid color background can potentially save memory bandwidth by > > setting the CRTC background property and using smaller planes to display > > the rest of the content. > > > > To avoid confusion between different ways of encoding RGB data, we > > define a standard 64-bit format that should be used for this property's > > value. Helper functions and macros are provided to generate and dissect > > values in this standard format with varying component precision values. > > > > v2: > > - Swap internal representation's blue and red bits to make it easier > >to read if printed out. (Ville) > > - Document bgcolor property in drm_blend.c. (Sean Paul) > > - s/background_color/bgcolor/ for consistency between property name and > >value storage field. (Sean Paul) > > - Add a convenience function to attach property to a given crtc. > > > > Cc: dri-de...@lists.freedesktop.org > > Cc: wei.c...@intel.com > > Cc: harish.krupo@intel.com > > Cc: Ville Syrjälä > > Cc: Sean Paul > > Signed-off-by: Matt Roper > > --- > > drivers/gpu/drm/drm_atomic_state_helper.c | 1 + > > drivers/gpu/drm/drm_atomic_uapi.c | 5 + > > drivers/gpu/drm/drm_blend.c | 21 ++--- > > drivers/gpu/drm/drm_mode_config.c | 6 ++ > > include/drm/drm_blend.h | 1 + > > include/drm/drm_crtc.h| 17 + > > include/drm/drm_mode_config.h | 5 + > > include/uapi/drm/drm_mode.h | 26 ++ > > 8 files changed, 79 insertions(+), 3 deletions(-) > > /snip > > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > > index d3e0fe31efc5..7c4f902aa290 100644 > > --- a/include/uapi/drm/drm_mode.h > > +++ b/include/uapi/drm/drm_mode.h > > @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease { > > __u32 lessee_id; > > }; > > > > +/* > > + * Put RGBA values into a standard 64-bit representation that can be used > > + * for ioctl parameters, inter-driver commmunication, etc. If the > > component > > + * values being provided contain less than 16 bits of precision, they'll > > + * be shifted into the most significant bits. > > + */ > > +static inline __u64 > > +drm_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha) > > +{ > > + int msb_shift = 16 - bpc; > > + > > + return (__u64)alpha << msb_shift << 48 | > > + (__u64)red << msb_shift << 32 | > > + (__u64)green << msb_shift << 16 | > > + (__u64)blue << msb_shift; > > +} > > + > > +/* > > + * Extract the specified number of bits of a specific color component from > > a > > + * standard 64-bit RGBA value. > > + */ > > +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xull) >> > > (16-numbits)) > > +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xull<<16) >> > > (32-numbits)) > > +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xull<<32) >> > > (48-numbits)) > > +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xull<<48) >> > > (64-numbits)) > > > #define DRM_RGBA_COMP(c, shift, numbits) \ > (__u16)(((c) & 0xull << (shift)) >> (32 - ((shift) + 16 - > (numbits))) I think this should just be #define DRM_RGBA_COMP(c, shift, numbits) \ (__u16)(((c) & 0xull << (shift)) >> ((shift) + 16 - (numbits))) right? I.e., right shift the same amount as the left shift, plus an additional (16-numbits) to account for the lower precision requested. Matt > > #define DRM_RGBA_BLUE(c, numbits) DRM_RGBA_COMP(c, 0, numbits) > #define DRM_RGBA_GREEN(c, numbits) DRM_RGBA_COMP(c, 16, numbits) > #define DRM_RGBA_RED(c, numbits) DRM_RGBA_COMP(c, 32, numbits) > #define DRM_RGBA_ALPHA(c, numbits) DRM_RGBA_COMP(c, 48, numbits) > > > With that, > > Reviewed-by: Sean Paul > > > > > + > > #if defined(__cplusplus) > > } > > #endif > > -- > > 2.14.4 > > > > -- > Sean Paul, Software Engineer, Google / Chromium OS -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/4] kms_content_protection: Fix log bug on 32-bit platforms.
== Series Details == Series: series starting with [1/4] kms_content_protection: Fix log bug on 32-bit platforms. URL : https://patchwork.freedesktop.org/series/52505/ State : success == Summary == = CI Bug Log - changes from IGT_4714 -> IGTPW_2062 = == Summary - WARNING == Minor unknown changes coming with IGTPW_2062 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_2062, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/52505/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in IGTPW_2062: === IGT changes === Possible regressions igt@kms_chamelium@hdmi-edid-read: {fi-kbl-7567u}: SKIP -> FAIL +3 Warnings igt@kms_chamelium@hdmi-hpd-fast: fi-icl-u2: SKIP -> PASS +3 igt@kms_flip@basic-flip-vs-modeset: fi-kbl-x1275: SKIP -> PASS +36 igt@prime_vgem@basic-fence-flip: fi-cfl-s3: SKIP -> PASS == Known issues == Here are the changes found in IGTPW_2062 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_contexts: fi-icl-u: NOTRUN -> INCOMPLETE (fdo#108315) +1 igt@drv_selftest@live_hangcheck: fi-bwr-2160:PASS -> DMESG-FAIL (fdo#108735) igt@kms_chamelium@common-hpd-after-suspend: fi-skl-6700k2: PASS -> WARN (fdo#108680) fi-icl-u2: SKIP -> DMESG-FAIL (fdo#103375, fdo#107732, fdo#108070) igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic: fi-icl-u2: PASS -> DMESG-WARN (fdo#107732) Possible fixes igt@drv_module_reload@basic-reload: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-byt-clapper: FAIL (fdo#107362, fdo#103191) -> PASS Warnings igt@drv_selftest@live_contexts: fi-icl-u2: INCOMPLETE (fdo#108315) -> DMESG-FAIL (fdo#108569) {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107732 https://bugs.freedesktop.org/show_bug.cgi?id=107732 fdo#108070 https://bugs.freedesktop.org/show_bug.cgi?id=108070 fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315 fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569 fdo#108680 https://bugs.freedesktop.org/show_bug.cgi?id=108680 fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735 == Participating hosts (53 -> 47) == Additional (1): fi-icl-u Missing(7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 == Build changes == * IGT: IGT_4714 -> IGTPW_2062 CI_DRM_5106: 852b9329fbb6ea8bdbb3dac78328aae73d919305 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2062: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2062/ IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Testlist changes == +igt@v3d_get_bo_offset@create-get-offsets +igt@v3d_get_bo_offset@get-bad-handle +igt@v3d_get_param@base-params +igt@v3d_get_param@get-bad-flags +igt@v3d_get_param@get-bad-param +igt@v3d_mmap@mmap-bad-handle == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2062/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/4] Add v3d helper library
Just a few little ioctl wrappers that v3d tests will use. Signed-off-by: Eric Anholt --- lib/Makefile.sources | 2 + lib/drmtest.c| 3 + lib/drmtest.h| 1 + lib/igt_v3d.c| 130 +++ lib/igt_v3d.h| 46 +++ lib/meson.build | 1 + 6 files changed, 183 insertions(+) create mode 100644 lib/igt_v3d.c create mode 100644 lib/igt_v3d.h diff --git a/lib/Makefile.sources b/lib/Makefile.sources index e98989ff8ed9..808b9617eca2 100644 --- a/lib/Makefile.sources +++ b/lib/Makefile.sources @@ -107,6 +107,8 @@ lib_source_list = \ igt_syncobj.h \ igt_psr.c \ igt_psr.h \ + igt_v3d.c \ + igt_v3d.h \ $(NULL) .PHONY: version.h.tmp diff --git a/lib/drmtest.c b/lib/drmtest.c index fee9d33ad2a5..d2aa1c19154f 100644 --- a/lib/drmtest.c +++ b/lib/drmtest.c @@ -202,6 +202,7 @@ static const struct module { } modules[] = { { DRIVER_AMDGPU, "amdgpu" }, { DRIVER_INTEL, "i915", modprobe_i915 }, + { DRIVER_V3D, "v3d" }, { DRIVER_VC4, "vc4" }, { DRIVER_VGEM, "vgem" }, { DRIVER_VIRTIO, "virtio-gpu" }, @@ -340,6 +341,8 @@ static const char *chipset_to_str(int chipset) switch (chipset) { case DRIVER_INTEL: return "intel"; + case DRIVER_V3D: + return "v3d"; case DRIVER_VC4: return "vc4"; case DRIVER_VGEM: diff --git a/lib/drmtest.h b/lib/drmtest.h index 949865ee54dd..96ee517e2ec1 100644 --- a/lib/drmtest.h +++ b/lib/drmtest.h @@ -43,6 +43,7 @@ #define DRIVER_VGEM(1 << 2) #define DRIVER_VIRTIO (1 << 3) #define DRIVER_AMDGPU (1 << 4) +#define DRIVER_V3D (1 << 5) /* * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system * with vgem as well as a supported driver, you can end up with a diff --git a/lib/igt_v3d.c b/lib/igt_v3d.c new file mode 100644 index ..1a5ede1bd5fc --- /dev/null +++ b/lib/igt_v3d.c @@ -0,0 +1,130 @@ +/* + * Copyright © 2016 Broadcom + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "drmtest.h" +#include "igt_aux.h" +#include "igt_core.h" +#include "igt_v3d.h" +#include "ioctl_wrappers.h" +#include "intel_reg.h" +#include "intel_chipset.h" +#include "v3d_drm.h" + +#if NEW_CONTEXT_PARAM_NO_ERROR_CAPTURE_API +#define LOCAL_CONTEXT_PARAM_NO_ERROR_CAPTURE 0x4 +#endif + +/** + * SECTION:igt_v3d + * @short_description: V3D support library + * @title: V3D + * @include: igt.h + * + * This library provides various auxiliary helper functions for writing V3D + * tests. + */ + +struct v3d_bo * +igt_v3d_create_bo(int fd, size_t size) +{ + struct v3d_bo *bo = calloc(1, sizeof(*bo)); + + struct drm_v3d_create_bo create = { + .size = size, + }; + + do_ioctl(fd, DRM_IOCTL_V3D_CREATE_BO, ); + + bo->handle = create.handle; + bo->offset = create.offset; + bo->size = size; + + return bo; +} + +void +igt_v3d_free_bo(int fd, struct v3d_bo *bo) +{ + if (bo->map) + munmap(bo->map, bo->size); + gem_close(fd, bo->handle); + free(bo); +} + +uint32_t +igt_v3d_get_bo_offset(int fd, uint32_t handle) +{ + struct drm_v3d_get_bo_offset get = { + .handle = handle, + }; + + do_ioctl(fd, DRM_IOCTL_V3D_GET_BO_OFFSET, ); + + return get.offset; +} + +uint32_t +igt_v3d_get_param(int fd, enum drm_v3d_param param) +{ + struct drm_v3d_get_param get = { + .param = param, + }; + + do_ioctl(fd, DRM_IOCTL_V3D_GET_PARAM, ); + + return get.value; +} + +void * +igt_v3d_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned prot) +{ +
[Intel-gfx] [PATCH i-g-t 4/4] igt/v3d_*: Add new tests for the V3D UABI.
These are basic non-rendering tests of the UABI. Signed-off-by: Eric Anholt --- lib/igt_v3d.c | 4 -- tests/Makefile.am | 2 + tests/Makefile.sources| 6 +++ tests/meson.build | 3 ++ tests/v3d_ci/README | 26 + tests/v3d_ci/v3d.testlist | 6 +++ tests/v3d_get_bo_offset.c | 78 ++ tests/v3d_get_param.c | 80 +++ tests/v3d_mmap.c | 55 +++ 9 files changed, 256 insertions(+), 4 deletions(-) create mode 100644 tests/v3d_ci/README create mode 100644 tests/v3d_ci/v3d.testlist create mode 100644 tests/v3d_get_bo_offset.c create mode 100644 tests/v3d_get_param.c create mode 100644 tests/v3d_mmap.c diff --git a/lib/igt_v3d.c b/lib/igt_v3d.c index 1a5ede1bd5fc..619c072c0e47 100644 --- a/lib/igt_v3d.c +++ b/lib/igt_v3d.c @@ -40,10 +40,6 @@ #include "intel_chipset.h" #include "v3d_drm.h" -#if NEW_CONTEXT_PARAM_NO_ERROR_CAPTURE_API -#define LOCAL_CONTEXT_PARAM_NO_ERROR_CAPTURE 0x4 -#endif - /** * SECTION:igt_v3d * @short_description: V3D support library diff --git a/tests/Makefile.am b/tests/Makefile.am index 3d1ce0bc1af8..a6b2ba51ea4f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -14,6 +14,8 @@ if BUILD_VC4 TESTS_progs += $(VC4_TESTS) endif +TESTS_progs += $(V3D_TESTS) + if HAVE_CHAMELIUM TESTS_progs += \ kms_chamelium \ diff --git a/tests/Makefile.sources b/tests/Makefile.sources index d007ebc74ab9..3ed60e7c30c7 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -15,6 +15,12 @@ VC4_TESTS = \ vc4_wait_seqno \ $(NULL) +V3D_TESTS = \ + v3d_get_bo_offset \ + v3d_get_param \ + v3d_mmap \ + $(NULL) + AMDGPU_TESTS = \ amdgpu/amd_basic \ amdgpu/amd_cs_nop \ diff --git a/tests/meson.build b/tests/meson.build index 3020f7984d7a..4472536aef65 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -85,6 +85,9 @@ test_progs = [ 'syncobj_wait', 'template', 'tools_test', + 'v3d_get_bo_offset', + 'v3d_get_param', + 'v3d_mmap', 'vc4_create_bo', 'vc4_dmabuf_poll', 'vc4_label_bo', diff --git a/tests/v3d_ci/README b/tests/v3d_ci/README new file mode 100644 index ..e03c552fb972 --- /dev/null +++ b/tests/v3d_ci/README @@ -0,0 +1,26 @@ +This directory contains test lists to be used for v3d's DRM support. The files +are passed to piglit with the --test-list parameter directly. + +The test lists are contained in the IGT repository for several +reasons: + +- The lists stay synchronized with the IGT codebase. +- Public availability. Kernel developers can see what tests are run, + and can see what changes are done to the set, when, and why. +- Explicit test lists in general make it possible to implement a new + test without having it run by everyone else before the tests and / or setup + are ready for it. + +Changing the test lists should only happen with approval from the v3d +maintainer, Eric Anholt (e...@anholt.net). + + +v3d.testlist + + +This test list is meant as a general test suite without any time +restriction for the v3d DRM driver, combining generic DRM and KMS +tests. As a reminder, you can run this with the meson build using: + +./build/runner/igt_runner --test-list tests/v3d_ci/v3d.testlist \ + build/tests -o results diff --git a/tests/v3d_ci/v3d.testlist b/tests/v3d_ci/v3d.testlist new file mode 100644 index ..b55e8e571d7d --- /dev/null +++ b/tests/v3d_ci/v3d.testlist @@ -0,0 +1,6 @@ +igt@v3d_get_bo_offset@create-get-offsets +igt@v3d_get_bo_offset@get-bad-handle +igt@v3d_get_param@base-params +igt@v3d_get_param@get-bad-param +igt@v3d_get_param@get-bad-flags +igt@v3d_mmap@mmap-bad-handle diff --git a/tests/v3d_get_bo_offset.c b/tests/v3d_get_bo_offset.c new file mode 100644 index ..0923dc85f0d0 --- /dev/null +++ b/tests/v3d_get_bo_offset.c @@ -0,0 +1,78 @@ +/* + * Copyright © 2017 Broadcom + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
[Intel-gfx] [PATCH i-g-t 1/4] kms_content_protection: Fix log bug on 32-bit platforms.
long is different between 32 and 64 and should basically never be used. Fixes compiler warning about passing the wrong type. Signed-off-by: Eric Anholt --- tests/kms_content_protection.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/kms_content_protection.c b/tests/kms_content_protection.c index 801eff66c272..bb9ecd3f4cde 100644 --- a/tests/kms_content_protection.c +++ b/tests/kms_content_protection.c @@ -89,7 +89,8 @@ wait_for_prop_value(igt_output_t *output, uint64_t expected, return true; usleep(1000); } - igt_info("prop_value mismatch %ld != %ld\n", val, expected); + igt_info("prop_value mismatch %lld != %lld\n", +(long long)val, (long long)expected); return false; } -- 2.19.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/4] v3d: Import the V3D DRM UAPI header.
Copied from make headers_install at drm-misc-next 783195ec1cad ("drm/syncobj: disable the timeline UAPI for now v2") Signed-off-by: Eric Anholt --- include/drm-uapi/v3d_drm.h | 204 + 1 file changed, 204 insertions(+) create mode 100644 include/drm-uapi/v3d_drm.h diff --git a/include/drm-uapi/v3d_drm.h b/include/drm-uapi/v3d_drm.h new file mode 100644 index ..f446656d00b1 --- /dev/null +++ b/include/drm-uapi/v3d_drm.h @@ -0,0 +1,204 @@ +/* + * Copyright © 2014-2018 Broadcom + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#ifndef _V3D_DRM_H_ +#define _V3D_DRM_H_ + +#include "drm.h" + +#if defined(__cplusplus) +extern "C" { +#endif + +#define DRM_V3D_SUBMIT_CL 0x00 +#define DRM_V3D_WAIT_BO 0x01 +#define DRM_V3D_CREATE_BO 0x02 +#define DRM_V3D_MMAP_BO 0x03 +#define DRM_V3D_GET_PARAM 0x04 +#define DRM_V3D_GET_BO_OFFSET 0x05 + +#define DRM_IOCTL_V3D_SUBMIT_CL DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_CL, struct drm_v3d_submit_cl) +#define DRM_IOCTL_V3D_WAIT_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_WAIT_BO, struct drm_v3d_wait_bo) +#define DRM_IOCTL_V3D_CREATE_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_CREATE_BO, struct drm_v3d_create_bo) +#define DRM_IOCTL_V3D_MMAP_BO DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_MMAP_BO, struct drm_v3d_mmap_bo) +#define DRM_IOCTL_V3D_GET_PARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_GET_PARAM, struct drm_v3d_get_param) +#define DRM_IOCTL_V3D_GET_BO_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_GET_BO_OFFSET, struct drm_v3d_get_bo_offset) + +/** + * struct drm_v3d_submit_cl - ioctl argument for submitting commands to the 3D + * engine. + * + * This asks the kernel to have the GPU execute an optional binner + * command list, and a render command list. + */ +struct drm_v3d_submit_cl { + /* Pointer to the binner command list. +* +* This is the first set of commands executed, which runs the +* coordinate shader to determine where primitives land on the screen, +* then writes out the state updates and draw calls necessary per tile +* to the tile allocation BO. +* +* This BCL will block on any previous BCL submitted on the +* same FD, but not on any RCL or BCLs submitted by other +* clients -- that is left up to the submitter to control +* using in_sync_bcl if necessary. +*/ + __u32 bcl_start; + +/** End address of the BCL (first byte after the BCL) */ + __u32 bcl_end; + + /* Offset of the render command list. +* +* This is the second set of commands executed, which will either +* execute the tiles that have been set up by the BCL, or a fixed set +* of tiles (in the case of RCL-only blits). +* +* This RCL will block on this submit's BCL, and any previous +* RCL submitted on the same FD, but not on any RCL or BCLs +* submitted by other clients -- that is left up to the +* submitter to control using in_sync_rcl if necessary. +*/ + __u32 rcl_start; + +/** End address of the RCL (first byte after the RCL) */ + __u32 rcl_end; + + /** An optional sync object to wait on before starting the BCL. */ + __u32 in_sync_bcl; + /** An optional sync object to wait on before starting the RCL. */ + __u32 in_sync_rcl; + /** An optional sync object to place the completion fence in. */ + __u32 out_sync; + + /* Offset of the tile alloc memory +* +* This is optional on V3D 3.3 (where the CL can set the value) but +* required on V3D 4.1. +*/ + __u32 qma; + + /** Size of the tile alloc memory. */ +
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Program SKL+ watermarks/ddb more carefully (rev7)
== Series Details == Series: drm/i915: Program SKL+ watermarks/ddb more carefully (rev7) URL : https://patchwork.freedesktop.org/series/51878/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5140 -> Patchwork_10827 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/51878/revisions/7/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10827: === IGT changes === Possible regressions igt@kms_chamelium@hdmi-edid-read: {fi-kbl-7567u}: SKIP -> FAIL +3 Warnings igt@kms_busy@basic-flip-a: {fi-kbl-7567u}: SKIP -> PASS +36 == Known issues == Here are the changes found in Patchwork_10827 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-reload-inject: fi-byt-clapper: PASS -> WARN (fdo#108688) igt@gem_ctx_create@basic-files: fi-icl-u2: PASS -> DMESG-WARN (fdo#107724) igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence: fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191) +1 igt@pm_rpm@module-reload: fi-byt-clapper: PASS -> FAIL (fdo#108675) Possible fixes igt@debugfs_test@read_all_entries: {fi-kbl-7567u}: DMESG-WARN (fdo#105602) -> PASS igt@drv_module_reload@basic-reload: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS igt@drv_module_reload@basic-reload-inject: {fi-kbl-7567u}: DMESG-WARN (fdo#105602, fdo#108529) -> PASS +1 igt@drv_selftest@live_hangcheck: fi-icl-u: INCOMPLETE (fdo#108315) -> PASS igt@gem_exec_suspend@basic-s3: {fi-kbl-7567u}: DMESG-WARN (fdo#105602, fdo#105079) -> PASS fi-icl-u2: DMESG-WARN (fdo#107724) -> PASS igt@kms_frontbuffer_tracking@basic: fi-icl-u2: FAIL (fdo#103167) -> PASS igt@pm_rpm@module-reload: {fi-kbl-7567u}: DMESG-WARN (fdo#108529) -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#105079 https://bugs.freedesktop.org/show_bug.cgi?id=105079 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724 fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315 fdo#108529 https://bugs.freedesktop.org/show_bug.cgi?id=108529 fdo#108675 https://bugs.freedesktop.org/show_bug.cgi?id=108675 fdo#108688 https://bugs.freedesktop.org/show_bug.cgi?id=108688 == Participating hosts (52 -> 47) == Missing(5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 == Build changes == * Linux: CI_DRM_5140 -> Patchwork_10827 CI_DRM_5140: 68c169139ed5a6ab2aa72f84a0a7dcd0f1576717 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10827: f21a10597270010e0c529eb87d911d9b4a4ec1c1 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == f21a10597270 drm/i915: Pass the plane to icl_program_input_csc_coeff() cb22c9ec683c drm/i915: Rename the confusing 'plane_id' to 'color_plane' 21fb8c0a9512 drm/i915: Commit skl+ planes in an order that avoids ddb overlaps b340dfdc7c50 drm/i915: Move ddb/wm programming into plane update/disable hooks on skl+ d52e2e45814c drm/i915: Don't pass dev_priv around so much 35935ddd1f7b drm/i915: Clean up skl+ vs. icl+ watermark computation 81ef2cb52f82 drm/i915: Pass the entire skl_plane_wm to skl_compute_transition_wm() 5217f4a8480a drm/i915: Remove some useless zeroing on skl+ wm calculations b38354b977bd drm/i915: Fix latency==0 handling for level 0 watermark on skl+ eae12eaba5ea drm/i915: Pass the new crtc_state to ->disable_plane() 29f4a4966a76 drm/i915: Introduce crtc_state->update_planes bitmask eb3c2f394649 drm/i915: Move single buffered plane register writes to the end 5a11dbcbd5cb drm/i915: Reorganize plane register writes to make them more atomic == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10827/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Program SKL+ watermarks/ddb more carefully (rev7)
== Series Details == Series: drm/i915: Program SKL+ watermarks/ddb more carefully (rev7) URL : https://patchwork.freedesktop.org/series/51878/ State : warning == Summary == $ dim sparse origin/drm-tip Sparse version: v0.5.2 Commit: drm/i915: Reorganize plane register writes to make them more atomic Okay! Commit: drm/i915: Move single buffered plane register writes to the end Okay! Commit: drm/i915: Introduce crtc_state->update_planes bitmask Okay! Commit: drm/i915: Pass the new crtc_state to ->disable_plane() Okay! Commit: drm/i915: Fix latency==0 handling for level 0 watermark on skl+ Okay! Commit: drm/i915: Remove some useless zeroing on skl+ wm calculations Okay! Commit: drm/i915: Pass the entire skl_plane_wm to skl_compute_transition_wm() Okay! Commit: drm/i915: Clean up skl+ vs. icl+ watermark computation Okay! Commit: drm/i915: Don't pass dev_priv around so much Okay! Commit: drm/i915: Move ddb/wm programming into plane update/disable hooks on skl+ -drivers/gpu/drm/i915/selftests/../i915_drv.h:3716:16: warning: expression using sizeof(void) +drivers/gpu/drm/i915/selftests/../i915_drv.h:3713:16: warning: expression using sizeof(void) Commit: drm/i915: Commit skl+ planes in an order that avoids ddb overlaps Okay! Commit: drm/i915: Rename the confusing 'plane_id' to 'color_plane' Okay! Commit: drm/i915: Pass the plane to icl_program_input_csc_coeff() Okay! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Program SKL+ watermarks/ddb more carefully (rev7)
== Series Details == Series: drm/i915: Program SKL+ watermarks/ddb more carefully (rev7) URL : https://patchwork.freedesktop.org/series/51878/ State : warning == Summary == $ dim checkpatch origin/drm-tip 5a11dbcbd5cb drm/i915: Reorganize plane register writes to make them more atomic eb3c2f394649 drm/i915: Move single buffered plane register writes to the end 29f4a4966a76 drm/i915: Introduce crtc_state->update_planes bitmask eae12eaba5ea drm/i915: Pass the new crtc_state to ->disable_plane() -:153: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__state' - possible side-effects? #153: FILE: drivers/gpu/drm/i915/intel_display.h:385: +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ + for ((__i) = 0; \ +(__i) < (__state)->base.dev->mode_config.num_total_plane && \ +((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ +(__i)++) \ + for_each_if(plane) -:153: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'plane' - possible side-effects? #153: FILE: drivers/gpu/drm/i915/intel_display.h:385: +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ + for ((__i) = 0; \ +(__i) < (__state)->base.dev->mode_config.num_total_plane && \ +((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ +(__i)++) \ + for_each_if(plane) -:153: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__i' - possible side-effects? #153: FILE: drivers/gpu/drm/i915/intel_display.h:385: +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ + for ((__i) = 0; \ +(__i) < (__state)->base.dev->mode_config.num_total_plane && \ +((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ +(__i)++) \ + for_each_if(plane) -:157: WARNING:LONG_LINE: line over 100 characters #157: FILE: drivers/gpu/drm/i915/intel_display.h:389: + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ total: 0 errors, 1 warnings, 3 checks, 159 lines checked b38354b977bd drm/i915: Fix latency==0 handling for level 0 watermark on skl+ 5217f4a8480a drm/i915: Remove some useless zeroing on skl+ wm calculations 81ef2cb52f82 drm/i915: Pass the entire skl_plane_wm to skl_compute_transition_wm() 35935ddd1f7b drm/i915: Clean up skl+ vs. icl+ watermark computation d52e2e45814c drm/i915: Don't pass dev_priv around so much b340dfdc7c50 drm/i915: Move ddb/wm programming into plane update/disable hooks on skl+ -:161: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__state' - possible side-effects? #161: FILE: drivers/gpu/drm/i915/intel_display.h:418: +#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ + for ((__i) = 0; \ +(__i) < (__state)->base.dev->mode_config.num_crtc && \ +((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \ + (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \ + (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \ +(__i)++) \ + for_each_if(crtc) -:161: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'crtc' - possible side-effects? #161: FILE: drivers/gpu/drm/i915/intel_display.h:418: +#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ + for ((__i) = 0; \ +(__i) < (__state)->base.dev->mode_config.num_crtc && \ +((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \ + (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \ + (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \ +(__i)++) \ + for_each_if(crtc) -:161: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__i' - possible side-effects? #161: FILE: drivers/gpu/drm/i915/intel_display.h:418: +#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ + for ((__i) = 0; \ +(__i) < (__state)->base.dev->mode_config.num_crtc && \ +((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \ + (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \ + (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \ +(__i)++) \ +
[Intel-gfx] [PATCH v2 11/13] drm/i915: Commit skl+ planes in an order that avoids ddb overlaps
From: Ville Syrjälä skl+ can go belly up if there are overlapping ddb allocations between planes. If we could absolutely guarantee that we can perform the atomic update within a single frame we shouldn't have to worry about this. But we can't rely on that so let's steal the ddb overlap check trick from skl_update_crtcs() and apply it to the plane updates. Since each step of the sequence is free from ddb overlaps we don't have to worry about a vblank sneaking up on us in the middle of the sequence. The partial state that gets latched by the hardware will be safe. And unlike skl_update_crtcs() we don't have to intoduce any extra vblank waits on accoung of only having to worry about a single pipe. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_atomic_plane.c | 96 --- drivers/gpu/drm/i915/intel_display.c | 7 +- drivers/gpu/drm/i915/intel_drv.h | 8 +- 3 files changed, 93 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 69fc7010190c..ff8d3e577bbf 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -169,24 +169,75 @@ static int intel_plane_atomic_check(struct drm_plane *plane, to_intel_plane_state(new_plane_state)); } -void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, -struct intel_crtc *crtc, -struct intel_crtc_state *old_crtc_state, -struct intel_crtc_state *new_crtc_state) +static struct intel_plane * +skl_next_plane_to_commit(struct intel_atomic_state *state, +struct intel_crtc *crtc, +struct skl_ddb_entry entries_y[I915_MAX_PLANES], +struct skl_ddb_entry entries_uv[I915_MAX_PLANES], +unsigned int *update_mask) { - u32 update_mask = new_crtc_state->update_planes; - struct intel_plane_state *new_plane_state; + struct intel_crtc_state *crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + struct intel_plane_state *plane_state; struct intel_plane *plane; int i; - for_each_new_intel_plane_in_state(old_state, plane, new_plane_state, i) { + if (*update_mask == 0) + return NULL; + + for_each_new_intel_plane_in_state(state, plane, plane_state, i) { + enum plane_id plane_id = plane->id; + if (crtc->pipe != plane->pipe || - !(update_mask & BIT(plane->id))) + !(*update_mask & BIT(plane_id))) continue; + if (skl_ddb_allocation_overlaps(_state->wm.skl.plane_ddb_y[plane_id], + entries_y, + I915_MAX_PLANES, plane_id) || + skl_ddb_allocation_overlaps(_state->wm.skl.plane_ddb_uv[plane_id], + entries_uv, + I915_MAX_PLANES, plane_id)) + continue; + + *update_mask &= ~BIT(plane_id); + entries_y[plane_id] = crtc_state->wm.skl.plane_ddb_y[plane_id]; + entries_uv[plane_id] = crtc_state->wm.skl.plane_ddb_uv[plane_id]; + + return plane; + } + + /* should never happen */ + WARN_ON(1); + + return NULL; +} + +void skl_update_planes_on_crtc(struct intel_atomic_state *state, + struct intel_crtc *crtc) +{ + struct intel_crtc_state *old_crtc_state = + intel_atomic_get_old_crtc_state(state, crtc); + struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + struct skl_ddb_entry entries_y[I915_MAX_PLANES]; + struct skl_ddb_entry entries_uv[I915_MAX_PLANES]; + u32 update_mask = new_crtc_state->update_planes; + struct intel_plane *plane; + + memcpy(entries_y, old_crtc_state->wm.skl.plane_ddb_y, + sizeof(old_crtc_state->wm.skl.plane_ddb_y)); + memcpy(entries_uv, old_crtc_state->wm.skl.plane_ddb_uv, + sizeof(old_crtc_state->wm.skl.plane_ddb_uv)); + + while ((plane = skl_next_plane_to_commit(state, crtc, +entries_y, entries_uv, +_mask))) { + struct intel_plane_state *new_plane_state = + intel_atomic_get_new_plane_state(state, plane); + if (new_plane_state->base.visible) { trace_intel_update_plane(>base, crtc); - plane->update_plane(plane, new_crtc_state, new_plane_state); } else if (new_plane_state->slave) {
[Intel-gfx] [PATCH v2 06/13] drm/i915: Remove some useless zeroing on skl+ wm calculations
From: Ville Syrjälä We memset(0) the entire watermark struct the start, so there's no need to clear things later on. v2: Rebase due to some stale w/a removal Reviewed-by: Rodrigo Vivi Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 16 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 25f589c4f68c..eb3ce3ee4df3 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4707,10 +4707,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if (latency == 0) return level == 0 ? -EINVAL : 0; - if (!intel_wm_plane_visible(cstate, intel_pstate)) { - result->plane_en = false; + if (!intel_wm_plane_visible(cstate, intel_pstate)) return 0; - } /* Display WA #1141: kbl,cfl */ if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || @@ -4807,8 +4805,6 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if ((level > 0 && res_lines > 31) || res_blocks >= ddb_allocation || min_disp_buf_needed >= ddb_allocation) { - result->plane_en = false; - /* * If there are no valid level 0 watermarks, then we can't * support this display configuration. @@ -4910,15 +4906,15 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, uint16_t wm0_sel_res_b, trans_offset_b, res_blocks; if (!cstate->base.active) - goto exit; + return; /* Transition WM are not recommended by HW team for GEN9 */ if (INTEL_GEN(dev_priv) <= 9) - goto exit; + return; /* Transition WM don't make any sense if ipc is disabled */ if (!dev_priv->ipc_enabled) - goto exit; + return; trans_min = 14; if (INTEL_GEN(dev_priv) >= 11) @@ -4957,11 +4953,7 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, if (res_blocks < ddb_allocation) { trans_wm->plane_res_b = res_blocks; trans_wm->plane_en = true; - return; } - -exit: - trans_wm->plane_en = false; } static int __skl_build_plane_wm_single(struct skl_ddb_allocation *ddb, -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 13/13] drm/i915: Pass the plane to icl_program_input_csc_coeff()
From: Ville Syrjälä On icl+ the plane state that gets passed to update_slave() is not the plane state of the plane we're programming. With NV12 the plane state would be coming from the master (UV) plane whereas the plane we're programming is the slave (Y) plane. For that reason we need to explicitly pass around the slave plane (or we'd have to otherwise deduce it by checking whether we were called via .update_plane() or .update_slave()). In the case of icl_program_input_csc_coeff() it's actually OK to assume that we are always the master plane because the input CSC only exists on HDR planes which can never be a slave plane. But for consistency let's pass in the plane explicitly anyway. While at it drop the "_coeff" from the function name since it's kinda redundant, and this makes the name a bit shorter :) Cc: Uma Shankar Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_sprite.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0262159e7084..ee4c37a613f7 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -373,14 +373,12 @@ skl_program_scaler(struct intel_plane *plane, #define BOFF(x) (((x) & 0x) << 16) static void -icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, - const struct intel_plane_state *plane_state) +icl_program_input_csc(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) { - struct drm_i915_private *dev_priv = - to_i915(plane_state->base.plane->dev); - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - enum pipe pipe = crtc->pipe; - struct intel_plane *plane = to_intel_plane(plane_state->base.plane); + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); + enum pipe pipe = plane->pipe; enum plane_id plane_id = plane->id; static const u16 input_csc_matrix[][9] = { @@ -540,7 +538,7 @@ skl_program_plane(struct intel_plane *plane, plane_state->color_ctl); if (fb->format->is_yuv && icl_is_hdr_plane(plane)) - icl_program_input_csc_coeff(crtc_state, plane_state); + icl_program_input_csc(plane, crtc_state, plane_state); skl_write_plane_wm(plane, crtc_state); -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 09/13] drm/i915: Don't pass dev_priv around so much
From: Ville Syrjälä Simplify the calling convention of the skl+ watermark functions by not passing around dev_priv needlessly. The callees have what they need to dig it out anyway. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 27 +-- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a743e089ab7d..a21654c974ba 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4578,12 +4578,12 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate, } static int -skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv, - const struct intel_crtc_state *cstate, +skl_compute_plane_wm_params(const struct intel_crtc_state *cstate, const struct intel_plane_state *intel_pstate, struct skl_wm_params *wp, int plane_id) { struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane); + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); const struct drm_plane_state *pstate = _pstate->base; const struct drm_framebuffer *fb = pstate->fb; uint32_t interm_pbpl; @@ -4682,8 +4682,7 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv, return 0; } -static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, - const struct intel_crtc_state *cstate, +static int skl_compute_plane_wm(const struct intel_crtc_state *cstate, const struct intel_plane_state *intel_pstate, uint16_t ddb_allocation, int level, @@ -4691,6 +4690,8 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, const struct skl_wm_level *result_prev, struct skl_wm_level *result /* out */) { + struct drm_i915_private *dev_priv = + to_i915(intel_pstate->base.plane->dev); const struct drm_plane_state *pstate = _pstate->base; uint32_t latency = dev_priv->wm.skl_latency[level]; uint_fixed_16_16_t method1, method2; @@ -4825,13 +4826,14 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, } static int -skl_compute_wm_levels(const struct drm_i915_private *dev_priv, - const struct intel_crtc_state *cstate, +skl_compute_wm_levels(const struct intel_crtc_state *cstate, const struct intel_plane_state *intel_pstate, uint16_t ddb_blocks, const struct skl_wm_params *wm_params, struct skl_wm_level *levels) { + struct drm_i915_private *dev_priv = + to_i915(intel_pstate->base.plane->dev); int level, max_level = ilk_wm_max_level(dev_priv); struct skl_wm_level *result_prev = [0]; int ret; @@ -4839,8 +4841,7 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv, for (level = 0; level <= max_level; level++) { struct skl_wm_level *result = [level]; - ret = skl_compute_plane_wm(dev_priv, - cstate, + ret = skl_compute_plane_wm(cstate, intel_pstate, ddb_blocks, level, @@ -4944,19 +4945,18 @@ static int skl_build_plane_wm_single(struct skl_ddb_allocation *ddb, enum plane_id plane_id, int color_plane) { struct intel_plane *plane = to_intel_plane(plane_state->base.plane); - struct drm_i915_private *dev_priv = to_i915(plane->base.dev); struct skl_plane_wm *wm = _state->wm.skl.optimal.planes[plane_id]; struct skl_wm_params wm_params; enum pipe pipe = plane->pipe; uint16_t ddb_blocks = skl_ddb_entry_size(>plane[pipe][plane_id]); int ret; - ret = skl_compute_plane_wm_params(dev_priv, crtc_state, plane_state, + ret = skl_compute_plane_wm_params(crtc_state, plane_state, _params, color_plane); if (ret) return ret; - ret = skl_compute_wm_levels(dev_priv, crtc_state, plane_state, + ret = skl_compute_wm_levels(crtc_state, plane_state, ddb_blocks, _params, wm->wm); if (ret) return ret; @@ -4972,7 +4972,6 @@ static int skl_build_plane_wm_uv(struct skl_ddb_allocation *ddb, enum plane_id plane_id) { struct intel_plane *plane = to_intel_plane(plane_state->base.plane); - struct drm_i915_private *dev_priv = to_i915(plane->base.dev); struct skl_plane_wm *wm =
[Intel-gfx] [PATCH v2 07/13] drm/i915: Pass the entire skl_plane_wm to skl_compute_transition_wm()
From: Ville Syrjälä We have to pass both level 0 watermark struct and the transition watermark struct to skl_compute_transition_wm(). Make life less confusing by just passing the entire plane watermark struct that contains both aforementioned structures. Cc: Matt Roper Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index eb3ce3ee4df3..59c91ec11c60 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4894,10 +4894,9 @@ skl_compute_linetime_wm(const struct intel_crtc_state *cstate) } static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, - struct skl_wm_params *wp, - struct skl_wm_level *wm_l0, - uint16_t ddb_allocation, - struct skl_wm_level *trans_wm /* out */) + const struct skl_wm_params *wp, + struct skl_plane_wm *wm, + uint16_t ddb_allocation) { struct drm_device *dev = cstate->base.crtc->dev; const struct drm_i915_private *dev_priv = to_i915(dev); @@ -4932,7 +4931,7 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, * Result Blocks is Result Blocks minus 1 and it should work for the * current platforms. */ - wm0_sel_res_b = wm_l0->plane_res_b - 1; + wm0_sel_res_b = wm->wm[0].plane_res_b - 1; if (wp->y_tiled) { trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2, @@ -4951,8 +4950,8 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, res_blocks += 1; if (res_blocks < ddb_allocation) { - trans_wm->plane_res_b = res_blocks; - trans_wm->plane_en = true; + wm->trans_wm.plane_res_b = res_blocks; + wm->trans_wm.plane_en = true; } } @@ -4981,8 +4980,7 @@ static int __skl_build_plane_wm_single(struct skl_ddb_allocation *ddb, if (ret) return ret; - skl_compute_transition_wm(cstate, _params, >wm[0], - ddb_blocks, >trans_wm); + skl_compute_transition_wm(cstate, _params, wm, ddb_blocks); return 0; } -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 12/13] drm/i915: Rename the confusing 'plane_id' to 'color_plane'
From: Ville Syrjälä A variable whose name is 'plane_id' is expected to be of the enum plane_id type. In this case we have a raw int, which turns out to refer to the plane of the framebuffer. Rename the variable to 'color_plane' in line with the trend started earlier. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1b337004054a..395c11b8a212 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4581,7 +4581,7 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate, static int skl_compute_plane_wm_params(const struct intel_crtc_state *cstate, const struct intel_plane_state *intel_pstate, - struct skl_wm_params *wp, int plane_id) + struct skl_wm_params *wp, int color_plane) { struct intel_plane *plane = to_intel_plane(intel_pstate->base.plane); struct drm_i915_private *dev_priv = to_i915(plane->base.dev); @@ -4593,7 +4593,7 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate, bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); /* only NV12 format has two planes */ - if (plane_id == 1 && fb->format->format != DRM_FORMAT_NV12) { + if (color_plane == 1 && fb->format->format != DRM_FORMAT_NV12) { DRM_DEBUG_KMS("Non NV12 format have single plane\n"); return -EINVAL; } @@ -4618,10 +4618,10 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *cstate, wp->width = drm_rect_width(_pstate->base.src) >> 16; } - if (plane_id == 1 && wp->is_planar) + if (color_plane == 1 && wp->is_planar) wp->width /= 2; - wp->cpp = fb->format->cpp[plane_id]; + wp->cpp = fb->format->cpp[color_plane]; wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate, intel_pstate); -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 05/13] drm/i915: Fix latency==0 handling for level 0 watermark on skl+
From: Ville Syrjälä If the level 0 latency is 0 we can't do anything. Return an error rather than success. While this can't happen due to WaWmMemoryReadLatency, it can happen if the user clears out the level 0 latency via debugfs. v2: Clarify how how we can end here with zero level 0 latency (Matt) Cc: Matt Roper Reviewed-by: Rodrigo Vivi Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 27498ded4949..25f589c4f68c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4704,8 +4704,10 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); uint32_t min_disp_buf_needed; - if (latency == 0 || - !intel_wm_plane_visible(cstate, intel_pstate)) { + if (latency == 0) + return level == 0 ? -EINVAL : 0; + + if (!intel_wm_plane_visible(cstate, intel_pstate)) { result->plane_en = false; return 0; } -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 02/13] drm/i915: Move single buffered plane register writes to the end
From: Ville Syrjälä The plane color correction registers are single buffered. So ideally we would write them at the start of vblank just after the double buffered plane registers have been latched. Since we have no convenient way to do that for now let's at least move the single buffered register writes to happen after the double buffered registers have been written. Reviewed-by: Rodrigo Vivi Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_sprite.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index a80773211265..6403ef2219d0 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -824,8 +824,6 @@ vlv_update_plane(struct intel_plane *plane, spin_lock_irqsave(_priv->uncore.lock, irqflags); - vlv_update_clrc(plane_state); - I915_WRITE_FW(SPSTRIDE(pipe, plane_id), plane_state->color_plane[0].stride); I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); @@ -853,6 +851,8 @@ vlv_update_plane(struct intel_plane *plane, I915_WRITE_FW(SPSURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + sprsurf_offset); + vlv_update_clrc(plane_state); + spin_unlock_irqrestore(_priv->uncore.lock, irqflags); } -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 10/13] drm/i915: Move ddb/wm programming into plane update/disable hooks on skl+
From: Ville Syrjälä On SKL+ the plane WM/BUF_CFG registers are a proper part of each plane's register set. That means accessing them will cancel any pending plane update, and we would need a PLANE_SURF register write to arm the wm/ddb change as well. To avoid all the problems with that let's just move the wm/ddb programming into the plane update/disable hooks. Now all plane registers get written in one (hopefully atomic) operation. To make that feasible we'll move the plane ddb tracking into the crtc state. Watermarks were already tracked there. v2: Rebase due to input CSC v3: Split out a bunch of junk (Matt) Cc: Matt Roper Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/i915_debugfs.c | 21 +- drivers/gpu/drm/i915/i915_drv.h | 3 - drivers/gpu/drm/i915/intel_display.c | 16 +- drivers/gpu/drm/i915/intel_display.h | 11 +- drivers/gpu/drm/i915/intel_drv.h | 9 + drivers/gpu/drm/i915/intel_pm.c | 317 --- drivers/gpu/drm/i915/intel_sprite.c | 4 + 7 files changed, 181 insertions(+), 200 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 670db5073d70..f8b2200947cf 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3437,31 +3437,32 @@ static int i915_ddb_info(struct seq_file *m, void *unused) { struct drm_i915_private *dev_priv = node_to_i915(m->private); struct drm_device *dev = _priv->drm; - struct skl_ddb_allocation *ddb; struct skl_ddb_entry *entry; - enum pipe pipe; - int plane; + struct intel_crtc *crtc; if (INTEL_GEN(dev_priv) < 9) return -ENODEV; drm_modeset_lock_all(dev); - ddb = _priv->wm.skl_hw.ddb; - seq_printf(m, "%-15s%8s%8s%8s\n", "", "Start", "End", "Size"); - for_each_pipe(dev_priv, pipe) { + for_each_intel_crtc(_priv->drm, crtc) { + struct intel_crtc_state *crtc_state = + to_intel_crtc_state(crtc->base.state); + enum pipe pipe = crtc->pipe; + enum plane_id plane_id; + seq_printf(m, "Pipe %c\n", pipe_name(pipe)); - for_each_universal_plane(dev_priv, pipe, plane) { - entry = >plane[pipe][plane]; - seq_printf(m, " Plane%-8d%8u%8u%8u\n", plane + 1, + for_each_plane_id_on_crtc(crtc, plane_id) { + entry = _state->wm.skl.plane_ddb_y[plane_id]; + seq_printf(m, " Plane%-8d%8u%8u%8u\n", plane_id + 1, entry->start, entry->end, skl_ddb_entry_size(entry)); } - entry = >plane[pipe][PLANE_CURSOR]; + entry = _state->wm.skl.plane_ddb_y[PLANE_CURSOR]; seq_printf(m, " %-13s%8u%8u%8u\n", "Cursor", entry->start, entry->end, skl_ddb_entry_size(entry)); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5d686b585a95..89af64fe90a5 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1241,9 +1241,6 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1, } struct skl_ddb_allocation { - /* packed/y */ - struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; - struct skl_ddb_entry uv_plane[I915_MAX_PIPES][I915_MAX_PLANES]; u8 enabled_slices; /* GEN11 has configurable 2 slices */ }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0caba7258fee..2981cea3704a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10083,6 +10083,10 @@ static void i9xx_update_cursor(struct intel_plane *plane, * except when the plane is getting enabled at which time * the CURCNTR write arms the update. */ + + if (INTEL_GEN(dev_priv) >= 9) + skl_write_cursor_wm(plane, crtc_state); + if (plane->cursor.base != base || plane->cursor.size != fbc_ctl || plane->cursor.cntl != cntl) { @@ -11872,6 +11876,8 @@ static void verify_wm_state(struct drm_crtc *crtc, struct skl_pipe_wm hw_wm, *sw_wm; struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; + struct skl_ddb_entry hw_ddb_y[I915_MAX_PLANES]; + struct skl_ddb_entry hw_ddb_uv[I915_MAX_PLANES]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); const enum pipe pipe = intel_crtc->pipe; int plane, level, max_level = ilk_wm_max_level(dev_priv); @@ -11882,6 +11888,8 @@ static void verify_wm_state(struct drm_crtc *crtc, skl_pipe_wm_get_hw_state(crtc, _wm); sw_wm = _intel_crtc_state(new_state)->wm.skl.optimal; + skl_pipe_ddb_get_hw_state(intel_crtc,
[Intel-gfx] [PATCH v2 08/13] drm/i915: Clean up skl+ vs. icl+ watermark computation
From: Ville Syrjälä Make a cleaner split between the skl+ and icl+ ways of computing watermarks. This way skl_build_pipe_wm() doesn't have to know any of the gritty details of icl+ master/slave planes. We can also simplify a bunch of the lower level code by pulling the plane visibility checks a bit higher up. Cc: Matt Roper Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 192 +--- 1 file changed, 103 insertions(+), 89 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 59c91ec11c60..a743e089ab7d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4591,9 +4591,6 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv, to_intel_atomic_state(cstate->base.state); bool apply_memory_bw_wa = skl_needs_memory_bw_wa(state); - if (!intel_wm_plane_visible(cstate, intel_pstate)) - return 0; - /* only NV12 format has two planes */ if (plane_id == 1 && fb->format->format != DRM_FORMAT_NV12) { DRM_DEBUG_KMS("Non NV12 format have single plane\n"); @@ -4707,9 +4704,6 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, if (latency == 0) return level == 0 ? -EINVAL : 0; - if (!intel_wm_plane_visible(cstate, intel_pstate)) - return 0; - /* Display WA #1141: kbl,cfl */ if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv) || IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_B0)) && @@ -4832,21 +4826,16 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv, static int skl_compute_wm_levels(const struct drm_i915_private *dev_priv, - struct skl_ddb_allocation *ddb, const struct intel_crtc_state *cstate, const struct intel_plane_state *intel_pstate, uint16_t ddb_blocks, const struct skl_wm_params *wm_params, - struct skl_plane_wm *wm, struct skl_wm_level *levels) { int level, max_level = ilk_wm_max_level(dev_priv); struct skl_wm_level *result_prev = [0]; int ret; - if (WARN_ON(!intel_pstate->base.fb)) - return -EINVAL; - for (level = 0; level <= max_level; level++) { struct skl_wm_level *result = [level]; @@ -4864,9 +4853,6 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv, result_prev = result; } - if (intel_pstate->base.fb->format->format == DRM_FORMAT_NV12) - wm->is_planar = true; - return 0; } @@ -4904,9 +4890,6 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, const uint16_t trans_amount = 10; /* This is configurable amount */ uint16_t wm0_sel_res_b, trans_offset_b, res_blocks; - if (!cstate->base.active) - return; - /* Transition WM are not recommended by HW team for GEN9 */ if (INTEL_GEN(dev_priv) <= 9) return; @@ -4955,97 +4938,134 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, } } -static int __skl_build_plane_wm_single(struct skl_ddb_allocation *ddb, - struct skl_pipe_wm *pipe_wm, - enum plane_id plane_id, - const struct intel_crtc_state *cstate, - const struct intel_plane_state *pstate, - int color_plane) -{ - struct drm_i915_private *dev_priv = to_i915(pstate->base.plane->dev); - struct skl_plane_wm *wm = _wm->planes[plane_id]; - enum pipe pipe = to_intel_plane(pstate->base.plane)->pipe; - struct skl_wm_params wm_params; - uint16_t ddb_blocks = skl_ddb_entry_size(>plane[pipe][plane_id]); - int ret; - - ret = skl_compute_plane_wm_params(dev_priv, cstate, pstate, - _params, color_plane); - if (ret) - return ret; - - ret = skl_compute_wm_levels(dev_priv, ddb, cstate, pstate, - ddb_blocks, _params, wm, wm->wm); - - if (ret) - return ret; - - skl_compute_transition_wm(cstate, _params, wm, ddb_blocks); - - return 0; -} - static int skl_build_plane_wm_single(struct skl_ddb_allocation *ddb, -struct skl_pipe_wm *pipe_wm, -const struct intel_crtc_state *cstate, -const struct intel_plane_state *pstate) +struct intel_crtc_state *crtc_state, +const struct intel_plane_state *plane_state, +
[Intel-gfx] [PATCH v2 04/13] drm/i915: Pass the new crtc_state to ->disable_plane()
From: Ville Syrjälä We're going to need access to the new crtc state in ->disable_plane() for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass the crtc state down. We'll also try to make intel_crtc_disable_planes() do the right thing as much as it's possible. The fact that we don't have a separate crtc state for the disabled state when we're going to re-enable the crtc later means we might end up poking at a few extra planes in there. But that's harmless. I suppose one might argue that we wouldn't have to care about proper ddb/wm/csc/gamma if the pipe is going to permanently disable anyway, but the state checker probably cares so we should try our best to make sure everything is programmed correctly even in that case. v2: Fix the commit message a bit (Matt) Cc: Matt Roper Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 39 ++- drivers/gpu/drm/i915/intel_display.h | 8 + drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_sprite.c | 12 --- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 010269a12390..69fc7010190c 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -210,7 +210,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, } else { trace_intel_disable_plane(>base, crtc); - plane->disable_plane(plane, crtc); + plane->disable_plane(plane, new_crtc_state); } } } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 065c8befc6f8..0caba7258fee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2767,7 +2767,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc, intel_pre_disable_primary_noatomic(>base); trace_intel_disable_plane(>base, crtc); - plane->disable_plane(plane, crtc); + plane->disable_plane(plane, crtc_state); } static void @@ -3372,7 +3372,7 @@ static void i9xx_update_plane(struct intel_plane *plane, } static void i9xx_disable_plane(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; @@ -5405,23 +5405,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, intel_update_watermarks(crtc); } -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask) +static void intel_crtc_disable_planes(struct intel_atomic_state *state, + struct intel_crtc *crtc) { - struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + const struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + unsigned int update_mask = new_crtc_state->update_planes; + const struct intel_plane_state *old_plane_state; struct intel_plane *plane; unsigned fb_bits = 0; + int i; intel_crtc_dpms_overlay_disable(crtc); - for_each_intel_plane_on_crtc(dev, crtc, plane) { - if (plane_mask & BIT(plane->id)) { - plane->disable_plane(plane, crtc); + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { + if (crtc->pipe != plane->pipe || + !(update_mask & BIT(plane->id))) + continue; + plane->disable_plane(plane, new_crtc_state); + + if (old_plane_state->base.visible) fb_bits |= plane->frontbuffer_bit; - } } - intel_frontbuffer_flip(to_i915(dev), fb_bits); + intel_frontbuffer_flip(dev_priv, fb_bits); } static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc, @@ -9866,9 +9875,9 @@ static void i845_update_cursor(struct intel_plane *plane, } static void i845_disable_cursor(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { - i845_update_cursor(plane, NULL, NULL); + i845_update_cursor(plane, crtc_state, NULL); } static bool i845_cursor_get_hw_state(struct intel_plane *plane, @@ -10095,9 +10104,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, } static void i9xx_disable_cursor(struct intel_plane *plane, - struct intel_crtc *crtc) +
[Intel-gfx] [PATCH v2 03/13] drm/i915: Introduce crtc_state->update_planes bitmask
From: Ville Syrjälä Keep track which planes need updating during the commit. For now this is just (was_visible || is_visible) but I'll have need to update invisible planes later on for skl plane ddbs and for pre-skl pipe gamma/csc control (which lives in the primary plane control register). Reviewed-by: Rodrigo Vivi Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_atomic_plane.c | 8 drivers/gpu/drm/i915/intel_display.c | 5 - drivers/gpu/drm/i915/intel_drv.h | 3 +++ 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index a5a2c8fe58a7..8cb02f28d30c 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -184,6 +184,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc) crtc_state->fifo_changed = false; crtc_state->wm.need_postvbl_update = false; crtc_state->fb_bits = 0; + crtc_state->update_planes = 0; return _state->base; } diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 7d3685075201..010269a12390 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -137,6 +137,9 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_ if (state->visible && state->fb->format->format == DRM_FORMAT_NV12) crtc_state->nv12_planes |= BIT(intel_plane->id); + if (state->visible || old_plane_state->base.visible) + crtc_state->update_planes |= BIT(intel_plane->id); + return intel_plane_atomic_calc_changes(old_crtc_state, _state->base, old_plane_state, @@ -171,14 +174,11 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, struct intel_crtc_state *old_crtc_state, struct intel_crtc_state *new_crtc_state) { + u32 update_mask = new_crtc_state->update_planes; struct intel_plane_state *new_plane_state; struct intel_plane *plane; - u32 update_mask; int i; - update_mask = old_crtc_state->active_planes; - update_mask |= new_crtc_state->active_planes; - for_each_new_intel_plane_in_state(old_state, plane, new_plane_state, i) { if (crtc->pipe != plane->pipe || !(update_mask & BIT(plane->id))) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3c760a2eacc8..065c8befc6f8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10808,8 +10808,10 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state) continue; plane_state->linked_plane = NULL; - if (plane_state->slave && !plane_state->base.visible) + if (plane_state->slave && !plane_state->base.visible) { crtc_state->active_planes &= ~BIT(plane->id); + crtc_state->update_planes |= BIT(plane->id); + } plane_state->slave = false; } @@ -10850,6 +10852,7 @@ static int icl_check_nv12_planes(struct intel_crtc_state *crtc_state) linked_state->slave = true; linked_state->linked_plane = plane; crtc_state->active_planes |= BIT(linked->id); + crtc_state->update_planes |= BIT(linked->id); DRM_DEBUG_KMS("Using %s as Y plane for %s\n", linked->base.name, plane->base.name); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 18b419f7f7fe..b0a24a81780a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -925,6 +925,9 @@ struct intel_crtc_state { u8 active_planes; u8 nv12_planes; + /* bitmask of planes that will be updated during the commit */ + u8 update_planes; + /* HDMI scrambling status */ bool hdmi_scrambling; -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 00/13] drm/i915: Program SKL+ watermarks/ddb more carefully
From: Ville Syrjälä Here's the remainder of the skl+ ddb/wm programming series. I tried to split up the ugly monster patch into a few chunks, and I tossed in a few extra nuggets on top. I also tried to improve the commit messages a bit based on the previous review feedback. Entire series available here: git://github.com/vsyrjala/linux.git skl_plane_ddb_wm_update_3 Ville Syrjälä (13): drm/i915: Reorganize plane register writes to make them more atomic drm/i915: Move single buffered plane register writes to the end drm/i915: Introduce crtc_state->update_planes bitmask drm/i915: Pass the new crtc_state to ->disable_plane() drm/i915: Fix latency==0 handling for level 0 watermark on skl+ drm/i915: Remove some useless zeroing on skl+ wm calculations drm/i915: Pass the entire skl_plane_wm to skl_compute_transition_wm() drm/i915: Clean up skl+ vs. icl+ watermark computation drm/i915: Don't pass dev_priv around so much drm/i915: Move ddb/wm programming into plane update/disable hooks on skl+ drm/i915: Commit skl+ planes in an order that avoids ddb overlaps drm/i915: Rename the confusing 'plane_id' to 'color_plane' drm/i915: Pass the plane to icl_program_input_csc_coeff() drivers/gpu/drm/i915/i915_debugfs.c | 21 +- drivers/gpu/drm/i915/i915_drv.h | 3 - drivers/gpu/drm/i915/intel_atomic.c | 1 + drivers/gpu/drm/i915/intel_atomic_plane.c | 102 - drivers/gpu/drm/i915/intel_display.c | 119 +++-- drivers/gpu/drm/i915/intel_display.h | 19 +- drivers/gpu/drm/i915/intel_drv.h | 22 +- drivers/gpu/drm/i915/intel_pm.c | 500 ++ drivers/gpu/drm/i915/intel_sprite.c | 148 --- 9 files changed, 523 insertions(+), 412 deletions(-) -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 01/13] drm/i915: Reorganize plane register writes to make them more atomic
From: Ville Syrjälä Some observations about the plane registers: - the control register will self-arm if the plane is not already enabled, thus we want to write it as close to (or ideally after) the surface register - tileoff/linoff/offset/aux_offset are self-arming as well so we want them close to the surface register as well - color keying registers we maybe self arming before SKL. Not 100% sure but we can try to keep them near to the surface register as well - chv pipe b csc register are double buffered but self arming so moving them down a bit - the rest should be mostly armed by the surface register so we can safely write them first, and to just for some consistency let's try to follow keep them in order based on the register offset None of this will have any effect of course unless the vblank evasion fails (which it still does sometimes). Another potential future benefit might be pulling the non-self armings registers outside the vblank evasion since they won't latch until the arming register has been written. This would make the critical section a bit lighter and thus less likely to exceed the deadline. v2: Rebase due to input CSC v3: Swap LINOFF/TILEOFF and KEYMSK/KEYMAX to actually follow the last rule above (Matt) Add a bit more rationale to the commit message (Matt) Cc: Matt Roper Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_display.c | 52 ++-- drivers/gpu/drm/i915/intel_sprite.c | 118 --- 2 files changed, 97 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 132e978227fb..3c760a2eacc8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3314,7 +3314,6 @@ static void i9xx_update_plane(struct intel_plane *plane, enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; u32 linear_offset; u32 dspcntr = plane_state->ctl; - i915_reg_t reg = DSPCNTR(i9xx_plane); int x = plane_state->color_plane[0].x; int y = plane_state->color_plane[0].y; unsigned long irqflags; @@ -3329,41 +3328,45 @@ static void i9xx_update_plane(struct intel_plane *plane, spin_lock_irqsave(_priv->uncore.lock, irqflags); + I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); + if (INTEL_GEN(dev_priv) < 4) { /* pipesrc and dspsize control the size that is scaled from, * which should always be the user's requested size. */ - I915_WRITE_FW(DSPSIZE(i9xx_plane), - ((crtc_state->pipe_src_h - 1) << 16) | - (crtc_state->pipe_src_w - 1)); I915_WRITE_FW(DSPPOS(i9xx_plane), 0); + I915_WRITE_FW(DSPSIZE(i9xx_plane), + ((crtc_state->pipe_src_h - 1) << 16) | + (crtc_state->pipe_src_w - 1)); } else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) { - I915_WRITE_FW(PRIMSIZE(i9xx_plane), - ((crtc_state->pipe_src_h - 1) << 16) | - (crtc_state->pipe_src_w - 1)); I915_WRITE_FW(PRIMPOS(i9xx_plane), 0); + I915_WRITE_FW(PRIMSIZE(i9xx_plane), + ((crtc_state->pipe_src_h - 1) << 16) | + (crtc_state->pipe_src_w - 1)); I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0); } - I915_WRITE_FW(reg, dspcntr); - - I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { - I915_WRITE_FW(DSPSURF(i9xx_plane), - intel_plane_ggtt_offset(plane_state) + - dspaddr_offset); I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x); } else if (INTEL_GEN(dev_priv) >= 4) { - I915_WRITE_FW(DSPSURF(i9xx_plane), - intel_plane_ggtt_offset(plane_state) + - dspaddr_offset); - I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset); - } else { + I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); + } + + /* +* The control register self-arms if the plane was previously +* disabled. Try to make the plane enable atomic by writing +* the control register just before the surface register. +*/ + I915_WRITE_FW(DSPCNTR(i9xx_plane), dspcntr); + if (INTEL_GEN(dev_priv) >= 4) + I915_WRITE_FW(DSPSURF(i9xx_plane), + intel_plane_ggtt_offset(plane_state) + + dspaddr_offset); + else I915_WRITE_FW(DSPADDR(i9xx_plane),
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: use appropriate integer types for flags
== Series Details == Series: drm/i915: use appropriate integer types for flags URL : https://patchwork.freedesktop.org/series/52481/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5138_full -> Patchwork_10824_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10824_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10824_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_10824_full: === IGT changes === Warnings igt@kms_lease@lease_invalid_connector: shard-snb: PASS -> SKIP +2 igt@perf_pmu@rc6: shard-kbl: SKIP -> PASS == Known issues == Here are the changes found in Patchwork_10824_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@forcewake: shard-kbl: PASS -> INCOMPLETE (fdo#103665) igt@gem_exec_schedule@pi-ringfull-bsd: shard-skl: NOTRUN -> FAIL (fdo#103158) igt@gem_exec_schedule@preempt-self-blt: shard-snb: NOTRUN -> INCOMPLETE (fdo#105411) igt@gem_ppgtt@blt-vs-render-ctxn: shard-skl: NOTRUN -> TIMEOUT (fdo#108039) igt@gem_userptr_blits@forbidden-operations: shard-apl: PASS -> INCOMPLETE (fdo#103927) igt@kms_atomic_transition@1x-modeset-transitions-nonblocking: shard-skl: NOTRUN -> FAIL (fdo#108228, fdo#108470) igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a: shard-skl: NOTRUN -> DMESG-WARN (fdo#107956) igt@kms_chv_cursor_fail@pipe-b-64x64-right-edge: shard-glk: PASS -> DMESG-FAIL (fdo#104671, fdo#106538) igt@kms_cursor_legacy@cursorb-vs-flipb-toggle: shard-glk: PASS -> DMESG-WARN (fdo#106538, fdo#105763) igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-skl: PASS -> FAIL (fdo#105363) igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw: shard-skl: NOTRUN -> FAIL (fdo#105682) +2 igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt: shard-glk: PASS -> FAIL (fdo#103167) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render: shard-apl: PASS -> FAIL (fdo#103167) +2 igt@kms_frontbuffer_tracking@fbc-1p-rte: shard-apl: PASS -> DMESG-WARN (fdo#105602, fdo#108131, fdo#103558) igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-render: shard-glk: PASS -> DMESG-FAIL (fdo#106538) igt@kms_frontbuffer_tracking@fbc-stridechange: shard-skl: NOTRUN -> FAIL (fdo#105683) igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite: shard-skl: NOTRUN -> FAIL (fdo#103167) +2 igt@kms_plane@pixel-format-pipe-c-planes: shard-skl: NOTRUN -> DMESG-WARN (fdo#106885) igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min: shard-skl: NOTRUN -> FAIL (fdo#108145) igt@kms_plane_multiple@atomic-pipe-a-tiling-x: shard-apl: PASS -> FAIL (fdo#103166) +1 igt@kms_properties@connector-properties-atomic: shard-glk: NOTRUN -> FAIL (fdo#108642) igt@kms_rotation_crc@primary-rotation-90: shard-skl: NOTRUN -> FAIL (fdo#107815, fdo#103925) Possible fixes igt@drv_suspend@forcewake: shard-skl: INCOMPLETE (fdo#107773, fdo#104108) -> PASS igt@gem_cpu_reloc@full: shard-skl: INCOMPLETE (fdo#108073) -> PASS igt@gem_userptr_blits@readonly-unsync: shard-skl: INCOMPLETE (fdo#108074) -> PASS igt@kms_busy@extended-modeset-hang-newfb-render-a: shard-hsw: DMESG-WARN (fdo#107956) -> PASS igt@kms_color@pipe-c-degamma: shard-apl: FAIL (fdo#104782) -> PASS igt@kms_cursor_crc@cursor-128x42-onscreen: shard-apl: FAIL (fdo#103232) -> PASS +1 igt@kms_cursor_crc@cursor-256x256-suspend: shard-skl: INCOMPLETE (fdo#104108) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite: shard-apl: FAIL (fdo#103167) -> PASS +1 igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff: shard-glk: FAIL (fdo#103167) -> PASS +1 igt@kms_frontbuffer_tracking@fbc-suspend: shard-kbl: FAIL (fdo#103375) -> PASS igt@kms_plane@pixel-format-pipe-c-planes: shard-apl: FAIL (fdo#103166) -> PASS +1 igt@kms_plane@plane-panning-bottom-right-pipe-b-planes: shard-skl: FAIL (fdo#103166) -> PASS igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max: shard-glk: FAIL (fdo#108145) -> PASS
[Intel-gfx] [PULL] drm-misc-fixes
Hi Dave, Hope you're having a good time in Vancouver, sorry I missed y'all! A bit more activity this week in -fixes, and that's all due to omap. We've got some initialization fixes, the most notable is migrating dss child instantiation from mach-* to the omapdss driver. Please pull at your convenience. drm-misc-fixes-2018-11-14: Cross-subsystem: - omap: Instantiate dss children in omapdss instead of mach (Laurent) Other: - htmldocs build warning (Sean) - MST NULL deref fix (Stanislav) - omap: Various runtime ref gets on probe/bind (Laurent) - omap: Fix to the above dss children patch (Tony) Cc: Sean Paul Cc: Stanislav Lisovskiy Cc: Laurent Pinchart Cc: Tony Lindgren Cheers, Sean The following changes since commit a8939766c75c06b5a0ab691ecbba9347e4e520cf: drm/sun4i: tcon: prevent tcon->panel dereference if NULL (2018-11-06 16:55:29 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2018-11-14 for you to fetch changes up to adf59dd2408c4536d490bee649784f0465562444: drm/meson: venc: dmt mode must use encp (2018-11-13 10:52:33 +0100) Cross-subsystem: - omap: Instantiate dss children in omapdss instead of mach (Laurent) Other: - htmldocs build warning (Sean) - MST NULL deref fix (Stanislav) - omap: Various runtime ref gets on probe/bind (Laurent) - omap: Fix to the above dss children patch (Tony) Cc: Sean Paul Cc: Stanislav Lisovskiy Cc: Laurent Pinchart Cc: Tony Lindgren Jorge Ramirez-Ortiz (1): drm/meson: venc: dmt mode must use encp Laurent Pinchart (4): drm/omap: Populate DSS children in omapdss driver drm/omap: hdmi4: Ensure the device is active during bind drm/omap: dsi: Ensure the device is active during probe drm/omap: Move DISPC runtime PM handling to omapdrm Sean Paul (1): drm: Fix htmldocs warnings in drm_fourcc.c Stanislav Lisovskiy (1): drm/dp_mst: Check if primary mstb is null Tony Lindgren (1): drm/omap: dsi: Fix missing of_platform_depopulate() arch/arm/mach-omap2/display.c | 111 -- drivers/gpu/drm/drm_dp_mst_topology.c | 3 + drivers/gpu/drm/drm_fourcc.c | 2 +- drivers/gpu/drm/meson/meson_venc.c| 15 ++--- drivers/gpu/drm/omapdrm/dss/dsi.c | 22 +++ drivers/gpu/drm/omapdrm/dss/dss.c | 11 +++- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 37 +++- drivers/gpu/drm/omapdrm/dss/hdmi5.c | 27 - drivers/gpu/drm/omapdrm/dss/venc.c| 7 --- drivers/gpu/drm/omapdrm/omap_crtc.c | 6 ++ 10 files changed, 101 insertions(+), 140 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Disable LP3 watermarks on all SNB machines (rev2)
== Series Details == Series: drm/i915: Disable LP3 watermarks on all SNB machines (rev2) URL : https://patchwork.freedesktop.org/series/52440/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5140 -> Patchwork_10826 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/52440/revisions/2/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10826: === IGT changes === Possible regressions igt@kms_chamelium@hdmi-edid-read: {fi-kbl-7567u}: SKIP -> FAIL +3 Warnings igt@kms_busy@basic-flip-a: {fi-kbl-7567u}: SKIP -> PASS +36 == Known issues == Here are the changes found in Patchwork_10826 that come from known issues: === IGT changes === Issues hit igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence: fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191) Possible fixes igt@debugfs_test@read_all_entries: {fi-kbl-7567u}: DMESG-WARN (fdo#105602) -> PASS igt@drv_module_reload@basic-reload: fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS igt@drv_module_reload@basic-reload-inject: {fi-kbl-7567u}: DMESG-WARN (fdo#108529, fdo#105602) -> PASS +1 igt@gem_exec_suspend@basic-s3: {fi-kbl-7567u}: DMESG-WARN (fdo#105602, fdo#105079) -> PASS igt@pm_rpm@module-reload: {fi-kbl-7567u}: DMESG-WARN (fdo#108529) -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#105079 https://bugs.freedesktop.org/show_bug.cgi?id=105079 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#108529 https://bugs.freedesktop.org/show_bug.cgi?id=108529 == Participating hosts (52 -> 45) == Missing(7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_5140 -> Patchwork_10826 CI_DRM_5140: 68c169139ed5a6ab2aa72f84a0a7dcd0f1576717 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10826: 87f29a20df8874ca71d5a926fceab6877da4e56b @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 87f29a20df88 drm/i915: Disable LP3 watermarks on all SNB machines == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10826/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/gen9+: Add support for pipe background color (v2)
On Tue, Nov 13, 2018 at 03:21:49PM -0800, Matt Roper wrote: > Gen9+ platforms allow CRTC's to be programmed with a background/canvas > color below the programmable planes. Let's expose this for use by > compositors. > > v2: > - Split out bgcolor sanitization and programming of csc/gamma bits to a >separate patch that we can land before the ABI changes are ready to >go in. (Ville) > - Change a temporary variable name to be more consistent with >other similar functions. (Ville) > - Change register name to SKL_CANVAS for consistency with the >CHV_CANVAS register. > > Cc: dri-de...@lists.freedesktop.org > Cc: wei.c...@intel.com > Cc: harish.krupo@intel.com > Cc: Ville Syrjälä > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/i915_debugfs.c | 9 + > drivers/gpu/drm/i915/intel_display.c | 35 --- > 2 files changed, 37 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index 670db5073d70..1f2a19e6ec79 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3254,6 +3254,15 @@ static int i915_display_info(struct seq_file *m, void > *unused) > intel_plane_info(m, crtc); > } > > + if (INTEL_GEN(dev_priv) >= 9 && pipe_config->base.active) { > + uint64_t background = pipe_config->base.bgcolor; > + > + seq_printf(m, "\tbackground color (10bpc): r=%x g=%x > b=%x\n", > +DRM_RGBA_RED(background, 10), > +DRM_RGBA_GREEN(background, 10), > +DRM_RGBA_BLUE(background, 10)); > + } > + > seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n", > yesno(!crtc->cpu_fifo_underrun_disabled), > yesno(!crtc->pch_fifo_underrun_disabled)); > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 1d089d93d88b..e7a759e0c021 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3834,6 +3834,27 @@ void intel_finish_reset(struct drm_i915_private > *dev_priv) > clear_bit(I915_RESET_MODESET, _priv->gpu_error.flags); > } > > +static void skl_update_background_color(const struct intel_crtc_state > *cstate) s/cstate/crtc_state/ please > +{ > + struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + uint64_t propval = cstate->base.bgcolor; > + uint32_t tmp; > + > + /* Hardware is programmed with 10 bits of precision */ > + tmp = DRM_RGBA_RED(propval, 10) << 20 > + | DRM_RGBA_GREEN(propval, 10) << 10 > + | DRM_RGBA_BLUE(propval, 10); > + > + /* > + * Set CSC and gamma for bottom color to ensure background pixels > + * receive the same color transformations as plane content. > + */ > + tmp |= SKL_CANVAS_CSC_ENABLE | SKL_CANVAS_GAMMA_ENABLE; > + > + I915_WRITE_FW(SKL_CANVAS(crtc->pipe), tmp); Why _FW? -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Force background color to black for gen9+
On Wed, Nov 14, 2018 at 09:36:39AM -0800, Matt Roper wrote: > On Wed, Nov 14, 2018 at 07:28:12PM +0200, Ville Syrjälä wrote: > > On Tue, Nov 13, 2018 at 03:21:47PM -0800, Matt Roper wrote: > > > We don't yet allow userspace to control the CRTC background color, but > > > we should manually program the color to black to ensure the BIOS didn't > > > leave us with some other color. We should also set the pipe gamma and > > > pipe CSC bits so that the background color goes through the same color > > > management transformations that a plane with black pixels would. > > > > > > Cc: Ville Syrjälä > > > Signed-off-by: Matt Roper > > > --- > > > We may want to land this patch before the rest of the series since it's > > > still valuable even without the new ABI the rest of the series adds. > > > > > > drivers/gpu/drm/i915/i915_reg.h | 6 ++ > > > drivers/gpu/drm/i915/intel_display.c | 18 ++ > > > 2 files changed, 24 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index fe4b913e46ac..b92a721c9bcb 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -5662,6 +5662,12 @@ enum { > > > #define PIPEMISC_DITHER_TYPE_SP(0 << 2) > > > #define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A) > > > > > > +/* Skylake+ pipe bottom (background) color */ > > > +#define _SKL_CANVAS_A0x70034 > > > +#define SKL_CANVAS_GAMMA_ENABLE(1 << 31) > > > +#define SKL_CANVAS_CSC_ENABLE (1 << 30) > > > +#define SKL_CANVAS(pipe) _MMIO_PIPE2(pipe, _SKL_CANVAS_A) > > > > Didn't the spec call this BOTTOM_COLOR or something like that? > > The register definition is called BOTTOM_COLOR, but other areas of the > spec refer to this as either "background color" or "canvas color." We > already have a CHV_CANVAS register that I believe provides the same > functionality for CHV (although I can't find details for the bit layout > in the spec), so I decided to just keep this name consistent with that > one. While CHV_CANVAS does provide simiar functionality (actually just the color, not the gamma/csc control) it is still a different register. IMO better to stick to the naming used in the spec. > > > Matt > > > > > > + > > > #define VLV_DPFLIPSTAT _MMIO(VLV_DISPLAY_BASE > > > + 0x70028) > > > #define PIPEB_LINE_COMPARE_INT_EN (1 << 29) > > > #define PIPEB_HLINE_INT_EN (1 << 28) > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 132e978227fb..1d089d93d88b 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -3868,6 +3868,15 @@ static void intel_update_pipe_config(const struct > > > intel_crtc_state *old_crtc_sta > > > else if (old_crtc_state->pch_pfit.enabled) > > > ironlake_pfit_disable(old_crtc_state); > > > } > > > + > > > + /* > > > + * We don't (yet) allow userspace to control the pipe background color, > > > + * so force it to black, but apply pipe gamma and CSC so that its > > > + * handling will match how we program our planes. > > > + */ > > > + if (INTEL_GEN(dev_priv) >= 9) > > > + I915_WRITE(SKL_CANVAS(crtc->pipe), > > > +SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE); > > > } > > > > > > static void intel_fdi_normal_train(struct intel_crtc *crtc) > > > @@ -15356,6 +15365,15 @@ static void intel_sanitize_crtc(struct > > > intel_crtc *crtc, > > > plane->base.type != DRM_PLANE_TYPE_PRIMARY) > > > intel_plane_disable_noatomic(crtc, plane); > > > } > > > + > > > + /* > > > + * Disable any background color set by the BIOS, but enable the > > > + * gamma and CSC to match how we program our planes. > > > + */ > > > + if (INTEL_GEN(dev_priv) >= 9) > > > + I915_WRITE(SKL_CANVAS(crtc->pipe), > > > +SKL_CANVAS_GAMMA_ENABLE | > > > +SKL_CANVAS_CSC_ENABLE); > > > } > > > > > > /* Adjust the state of the output pipe according to whether we > > > -- > > > 2.14.4 > > > > -- > > Ville Syrjälä > > Intel > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795 -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Force background color to black for gen9+
On Wed, Nov 14, 2018 at 07:28:12PM +0200, Ville Syrjälä wrote: > On Tue, Nov 13, 2018 at 03:21:47PM -0800, Matt Roper wrote: > > We don't yet allow userspace to control the CRTC background color, but > > we should manually program the color to black to ensure the BIOS didn't > > leave us with some other color. We should also set the pipe gamma and > > pipe CSC bits so that the background color goes through the same color > > management transformations that a plane with black pixels would. > > > > Cc: Ville Syrjälä > > Signed-off-by: Matt Roper > > --- > > We may want to land this patch before the rest of the series since it's > > still valuable even without the new ABI the rest of the series adds. > > > > drivers/gpu/drm/i915/i915_reg.h | 6 ++ > > drivers/gpu/drm/i915/intel_display.c | 18 ++ > > 2 files changed, 24 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index fe4b913e46ac..b92a721c9bcb 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -5662,6 +5662,12 @@ enum { > > #define PIPEMISC_DITHER_TYPE_SP (0 << 2) > > #define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A) > > > > +/* Skylake+ pipe bottom (background) color */ > > +#define _SKL_CANVAS_A 0x70034 > > +#define SKL_CANVAS_GAMMA_ENABLE (1 << 31) > > +#define SKL_CANVAS_CSC_ENABLE(1 << 30) > > +#define SKL_CANVAS(pipe) _MMIO_PIPE2(pipe, _SKL_CANVAS_A) > > Didn't the spec call this BOTTOM_COLOR or something like that? The register definition is called BOTTOM_COLOR, but other areas of the spec refer to this as either "background color" or "canvas color." We already have a CHV_CANVAS register that I believe provides the same functionality for CHV (although I can't find details for the bit layout in the spec), so I decided to just keep this name consistent with that one. Matt > > > + > > #define VLV_DPFLIPSTAT _MMIO(VLV_DISPLAY_BASE > > + 0x70028) > > #define PIPEB_LINE_COMPARE_INT_EN(1 << 29) > > #define PIPEB_HLINE_INT_EN (1 << 28) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 132e978227fb..1d089d93d88b 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3868,6 +3868,15 @@ static void intel_update_pipe_config(const struct > > intel_crtc_state *old_crtc_sta > > else if (old_crtc_state->pch_pfit.enabled) > > ironlake_pfit_disable(old_crtc_state); > > } > > + > > + /* > > +* We don't (yet) allow userspace to control the pipe background color, > > +* so force it to black, but apply pipe gamma and CSC so that its > > +* handling will match how we program our planes. > > +*/ > > + if (INTEL_GEN(dev_priv) >= 9) > > + I915_WRITE(SKL_CANVAS(crtc->pipe), > > + SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE); > > } > > > > static void intel_fdi_normal_train(struct intel_crtc *crtc) > > @@ -15356,6 +15365,15 @@ static void intel_sanitize_crtc(struct intel_crtc > > *crtc, > > plane->base.type != DRM_PLANE_TYPE_PRIMARY) > > intel_plane_disable_noatomic(crtc, plane); > > } > > + > > + /* > > +* Disable any background color set by the BIOS, but enable the > > +* gamma and CSC to match how we program our planes. > > +*/ > > + if (INTEL_GEN(dev_priv) >= 9) > > + I915_WRITE(SKL_CANVAS(crtc->pipe), > > + SKL_CANVAS_GAMMA_ENABLE | > > + SKL_CANVAS_CSC_ENABLE); > > } > > > > /* Adjust the state of the output pipe according to whether we > > -- > > 2.14.4 > > -- > Ville Syrjälä > Intel -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Disable LP3 watermarks on all SNB machines
From: Ville Syrjälä I have a Thinkpad X220 Tablet in my hands that is losing vblank interrupts whenever LP3 watermarks are used. If I nudge the latency value written to the WM3 register just by one in either direction the problem disappears. That to me suggests that the punit will not enter the corrsponding powersave mode (MPLL shutdown IIRC) unless the latency value in the register matches exactly what we read from SSKPD. Ie. it's not really a latency value but rather just a cookie by which the punit can identify the desired power saving state. On HSW/BDW this was changed such that we actually just write the WM level number into those bits, which makes much more sense given the observed behaviour. We could try to handle this by disallowing LP3 watermarks only when vblank interrupts are enabled but we'd first have to prove that only vblank interrupts are affected, which seems unlikely. Also we can't grab the wm mutex from the vblank enable/disable hooks because those are called with various spinlocks held. Thus we'd have to redesigne the watermark locking. So to play it safe and keep the code simple we simply disable LP3 watermarks on all SNB machines. To do that we simply zero out the latency values for watermark level 3, and we adjust the watermark computation to check for that. The behaviour now matches that of the g4x/vlv/skl wm code in the presence of a zeroed latency value. v2: s/USHRT_MAX/U32_MAX/ for consistency with the types (Chris) Cc: sta...@vger.kernel.org Cc: Chris Wilson Acked-by: Chris Wilson Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101269 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103713 Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/intel_pm.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 27498ded4949..897a791662c5 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2493,6 +2493,9 @@ static uint32_t ilk_compute_pri_wm(const struct intel_crtc_state *cstate, uint32_t method1, method2; int cpp; + if (mem_value == 0) + return U32_MAX; + if (!intel_wm_plane_visible(cstate, pstate)) return 0; @@ -2522,6 +2525,9 @@ static uint32_t ilk_compute_spr_wm(const struct intel_crtc_state *cstate, uint32_t method1, method2; int cpp; + if (mem_value == 0) + return U32_MAX; + if (!intel_wm_plane_visible(cstate, pstate)) return 0; @@ -2545,6 +2551,9 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate, { int cpp; + if (mem_value == 0) + return U32_MAX; + if (!intel_wm_plane_visible(cstate, pstate)) return 0; @@ -3008,6 +3017,34 @@ static void snb_wm_latency_quirk(struct drm_i915_private *dev_priv) intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency); } +static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv) +{ + /* +* On some SNB machines (Thinkpad X220 Tablet at least) +* LP3 usage can cause vblank interrupts to be lost. +* The DEIIR bit will go high but it looks like the CPU +* never gets interrupted. +* +* It's not clear whether other interrupt source could +* be affected or if this is somehow limited to vblank +* interrupts only. To play it safe we disable LP3 +* watermarks entirely. +*/ + if (dev_priv->wm.pri_latency[3] == 0 && + dev_priv->wm.spr_latency[3] == 0 && + dev_priv->wm.cur_latency[3] == 0) + return; + + dev_priv->wm.pri_latency[3] = 0; + dev_priv->wm.spr_latency[3] = 0; + dev_priv->wm.cur_latency[3] = 0; + + DRM_DEBUG_KMS("LP3 watermarks disabled due to potential for lost interrupts\n"); + intel_print_wm_latency(dev_priv, "Primary", dev_priv->wm.pri_latency); + intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency); + intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency); +} + static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv) { intel_read_wm_latency(dev_priv, dev_priv->wm.pri_latency); @@ -3024,8 +3061,10 @@ static void ilk_setup_wm_latency(struct drm_i915_private *dev_priv) intel_print_wm_latency(dev_priv, "Sprite", dev_priv->wm.spr_latency); intel_print_wm_latency(dev_priv, "Cursor", dev_priv->wm.cur_latency); - if (IS_GEN6(dev_priv)) + if (IS_GEN6(dev_priv)) { snb_wm_latency_quirk(dev_priv); + snb_wm_lp3_irq_quirk(dev_priv); + } } static void skl_setup_wm_latency(struct drm_i915_private *dev_priv) -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH v2 2/3] drm: Add CRTC background color property (v2)
On Wed, Nov 14, 2018 at 07:20:36PM +0200, Ville Syrjälä wrote: > On Tue, Nov 13, 2018 at 03:21:48PM -0800, Matt Roper wrote: ... > > > > +/* > > + * Put RGBA values into a standard 64-bit representation that can be used > > + * for ioctl parameters, inter-driver commmunication, etc. If the > > component > > + * values being provided contain less than 16 bits of precision, they'll > > + * be shifted into the most significant bits. > > + */ > > +static inline __u64 > > +drm_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha) > > +{ > > + int msb_shift = 16 - bpc; > > + > > + return (__u64)alpha << msb_shift << 48 | > > + (__u64)red << msb_shift << 32 | > > + (__u64)green << msb_shift << 16 | > > + (__u64)blue << msb_shift; > > So the value is now ARGB but everything still says RGBA? Well, the general idea was that you're providing four color components and you're not supposed to be looking behind the curtain at the internal representation (which may not match any given platform's hardware ordering). If everyone uses the helpers here to pack/unpack the components, then that avoids accidental differences in interpretation between different platforms. But I'm fine with changing the names; I'll do that when I incorporate Sean's suggestion. Matt > > > +} > > + > > +/* > > + * Extract the specified number of bits of a specific color component from > > a > > + * standard 64-bit RGBA value. > > + */ > > +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xull) >> > > (16-numbits)) > > +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xull<<16) >> > > (32-numbits)) > > +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xull<<32) >> > > (48-numbits)) > > +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xull<<48) >> > > (64-numbits)) > > + > > #if defined(__cplusplus) > > } > > #endif > > -- > > 2.14.4 > > -- > Ville Syrjälä > Intel -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Force background color to black for gen9+
On Tue, Nov 13, 2018 at 03:21:47PM -0800, Matt Roper wrote: > We don't yet allow userspace to control the CRTC background color, but > we should manually program the color to black to ensure the BIOS didn't > leave us with some other color. We should also set the pipe gamma and > pipe CSC bits so that the background color goes through the same color > management transformations that a plane with black pixels would. > > Cc: Ville Syrjälä > Signed-off-by: Matt Roper > --- > We may want to land this patch before the rest of the series since it's > still valuable even without the new ABI the rest of the series adds. > > drivers/gpu/drm/i915/i915_reg.h | 6 ++ > drivers/gpu/drm/i915/intel_display.c | 18 ++ > 2 files changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index fe4b913e46ac..b92a721c9bcb 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5662,6 +5662,12 @@ enum { > #define PIPEMISC_DITHER_TYPE_SP(0 << 2) > #define PIPEMISC(pipe) _MMIO_PIPE2(pipe, _PIPE_MISC_A) > > +/* Skylake+ pipe bottom (background) color */ > +#define _SKL_CANVAS_A0x70034 > +#define SKL_CANVAS_GAMMA_ENABLE(1 << 31) > +#define SKL_CANVAS_CSC_ENABLE (1 << 30) > +#define SKL_CANVAS(pipe) _MMIO_PIPE2(pipe, _SKL_CANVAS_A) Didn't the spec call this BOTTOM_COLOR or something like that? > + > #define VLV_DPFLIPSTAT _MMIO(VLV_DISPLAY_BASE > + 0x70028) > #define PIPEB_LINE_COMPARE_INT_EN (1 << 29) > #define PIPEB_HLINE_INT_EN (1 << 28) > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 132e978227fb..1d089d93d88b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3868,6 +3868,15 @@ static void intel_update_pipe_config(const struct > intel_crtc_state *old_crtc_sta > else if (old_crtc_state->pch_pfit.enabled) > ironlake_pfit_disable(old_crtc_state); > } > + > + /* > + * We don't (yet) allow userspace to control the pipe background color, > + * so force it to black, but apply pipe gamma and CSC so that its > + * handling will match how we program our planes. > + */ > + if (INTEL_GEN(dev_priv) >= 9) > + I915_WRITE(SKL_CANVAS(crtc->pipe), > +SKL_CANVAS_GAMMA_ENABLE | SKL_CANVAS_CSC_ENABLE); > } > > static void intel_fdi_normal_train(struct intel_crtc *crtc) > @@ -15356,6 +15365,15 @@ static void intel_sanitize_crtc(struct intel_crtc > *crtc, > plane->base.type != DRM_PLANE_TYPE_PRIMARY) > intel_plane_disable_noatomic(crtc, plane); > } > + > + /* > + * Disable any background color set by the BIOS, but enable the > + * gamma and CSC to match how we program our planes. > + */ > + if (INTEL_GEN(dev_priv) >= 9) > + I915_WRITE(SKL_CANVAS(crtc->pipe), > +SKL_CANVAS_GAMMA_ENABLE | > +SKL_CANVAS_CSC_ENABLE); > } > > /* Adjust the state of the output pipe according to whether we > -- > 2.14.4 -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2)
== Series Details == Series: series starting with [v2,1/2] drm/i915/icl: replace check for combo phy (rev2) URL : https://patchwork.freedesktop.org/series/52400/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5133_full -> Patchwork_10818_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10818_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10818_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_10818_full: === IGT changes === Warnings igt@kms_atomic_interruptible@universal-setplane-cursor: shard-snb: SKIP -> PASS +2 igt@pm_rc6_residency@rc6-accuracy: shard-kbl: SKIP -> PASS +1 == Known issues == Here are the changes found in Patchwork_10818_full that come from known issues: === IGT changes === Issues hit igt@gem_ppgtt@blt-vs-render-ctx0: shard-kbl: PASS -> INCOMPLETE (fdo#106887, fdo#103665, fdo#106023) igt@gem_userptr_blits@readonly-unsync: shard-skl: NOTRUN -> INCOMPLETE (fdo#108074) igt@kms_ccs@pipe-b-crc-sprite-planes-basic: shard-glk: PASS -> FAIL (fdo#108145) igt@kms_chv_cursor_fail@pipe-b-64x64-right-edge: shard-glk: PASS -> DMESG-FAIL (fdo#104671, fdo#106538) igt@kms_cursor_crc@cursor-256x256-onscreen: shard-skl: PASS -> FAIL (fdo#103232) igt@kms_cursor_crc@cursor-256x256-suspend: shard-apl: PASS -> FAIL (fdo#103191, fdo#103232) igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy: shard-hsw: PASS -> FAIL (fdo#105767) igt@kms_cursor_legacy@cursorb-vs-flipb-toggle: shard-glk: PASS -> DMESG-WARN (fdo#105763, fdo#106538) igt@kms_fbcon_fbt@fbc-suspend: shard-skl: PASS -> INCOMPLETE (fdo#104108, fdo#107773) igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-skl: PASS -> FAIL (fdo#105363) igt@kms_flip@plain-flip-ts-check: shard-skl: PASS -> FAIL (fdo#100368) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render: shard-apl: PASS -> FAIL (fdo#103167) igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-render: shard-glk: PASS -> DMESG-FAIL (fdo#106538) igt@kms_plane@plane-position-covered-pipe-b-planes: shard-glk: PASS -> FAIL (fdo#103166) igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb: shard-skl: NOTRUN -> FAIL (fdo#108145) +1 igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: shard-skl: PASS -> FAIL (fdo#107815) igt@kms_plane_multiple@atomic-pipe-a-tiling-x: shard-apl: PASS -> FAIL (fdo#103166) Possible fixes igt@drv_suspend@shrink: shard-skl: INCOMPLETE (fdo#106886) -> PASS shard-glk: INCOMPLETE (k.org#198133, fdo#103359, fdo#106886) -> PASS igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a: shard-skl: DMESG-WARN (fdo#107956) -> PASS igt@kms_ccs@pipe-a-crc-sprite-planes-basic: shard-glk: FAIL (fdo#108145) -> PASS igt@kms_chv_cursor_fail@pipe-c-256x256-top-edge: shard-skl: FAIL (fdo#104671) -> PASS igt@kms_color@pipe-c-ctm-blue-to-red: shard-skl: FAIL (fdo#107201) -> PASS igt@kms_cursor_crc@cursor-128x128-sliding: shard-apl: FAIL (fdo#103232) -> PASS igt@kms_flip@plain-flip-fb-recreate-interruptible: shard-skl: FAIL (fdo#100368) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu: shard-apl: FAIL (fdo#103167) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move: shard-glk: FAIL (fdo#103167) -> PASS +2 igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS shard-kbl: FAIL (fdo#99912) -> PASS igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend: shard-skl: INCOMPLETE (fdo#104108, fdo#107773) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 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#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108 fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671 fdo#105363
Re: [Intel-gfx] [PATCH v2 2/3] drm: Add CRTC background color property (v2)
On Tue, Nov 13, 2018 at 03:21:48PM -0800, Matt Roper wrote: > Some display controllers can be programmed to present non-black colors > for pixels not covered by any plane (or pixels covered by the > transparent regions of higher planes). Compositors that want a UI with > a solid color background can potentially save memory bandwidth by > setting the CRTC background property and using smaller planes to display > the rest of the content. > > To avoid confusion between different ways of encoding RGB data, we > define a standard 64-bit format that should be used for this property's > value. Helper functions and macros are provided to generate and dissect > values in this standard format with varying component precision values. > > v2: > - Swap internal representation's blue and red bits to make it easier >to read if printed out. (Ville) > - Document bgcolor property in drm_blend.c. (Sean Paul) > - s/background_color/bgcolor/ for consistency between property name and >value storage field. (Sean Paul) > - Add a convenience function to attach property to a given crtc. > > Cc: dri-de...@lists.freedesktop.org > Cc: wei.c...@intel.com > Cc: harish.krupo@intel.com > Cc: Ville Syrjälä > Cc: Sean Paul > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 5 + > drivers/gpu/drm/drm_blend.c | 21 ++--- > drivers/gpu/drm/drm_mode_config.c | 6 ++ > include/drm/drm_blend.h | 1 + > include/drm/drm_crtc.h| 17 + > include/drm/drm_mode_config.h | 5 + > include/uapi/drm/drm_mode.h | 26 ++ > 8 files changed, 79 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c > b/drivers/gpu/drm/drm_atomic_state_helper.c > index 3ba996069d69..2f8c55668089 100644 > --- a/drivers/gpu/drm/drm_atomic_state_helper.c > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c > @@ -101,6 +101,7 @@ void __drm_atomic_helper_crtc_duplicate_state(struct > drm_crtc *crtc, > state->planes_changed = false; > state->connectors_changed = false; > state->color_mgmt_changed = false; > + state->bgcolor_changed = false; Wondering a bit about the granulairty of these flags. Is bgcolor important enough to warrant its own flag, or should it just be part of color_mgmt_changed for example? color_mgmt_changed is rather heavy though as it would force LUT reprogramming, so I've been thinking that it should perhaps be split into separate gamma_lut_changed/degamma_lut_changed/other_color_stuff_changed flags. Anyways, just thinking out loud here. We can combine some of these flags later if we start to accumulate too many. > state->zpos_changed = false; > state->commit = NULL; > state->event = NULL; > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index 86ac33922b09..b95a55a778e2 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -467,6 +467,9 @@ static int drm_atomic_crtc_set_property(struct drm_crtc > *crtc, > return -EFAULT; > > set_out_fence_for_crtc(state->state, crtc, fence_ptr); > + } else if (property == config->bgcolor_property) { > + state->bgcolor = val; > + state->bgcolor_changed = true; > } else if (crtc->funcs->atomic_set_property) { > return crtc->funcs->atomic_set_property(crtc, state, property, > val); > } else { > @@ -499,6 +502,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, > *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0; > else if (property == config->prop_out_fence_ptr) > *val = 0; > + else if (property == config->bgcolor_property) > + *val = state->bgcolor; > else if (crtc->funcs->atomic_get_property) > return crtc->funcs->atomic_get_property(crtc, state, property, > val); > else > diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c > index 0c78ca386cbe..7c73cb83874a 100644 > --- a/drivers/gpu/drm/drm_blend.c > +++ b/drivers/gpu/drm/drm_blend.c > @@ -175,9 +175,16 @@ > *plane does not expose the "alpha" property, then this is > *assumed to be 1.0 > * > - * Note that all the property extensions described here apply either to the > - * plane or the CRTC (e.g. for the background color, which currently is not > - * exposed and assumed to be black). > + * The property extensions described above all apply to the plane. Drivers > + * may also expose the following crtc property extension: > + * > + * bgcolor: > + * Background color is setup with drm_crtc_add_bgcolor_property(). It > + * controls the RGB color of a full-screen, fully-opaque layer that exists > + * below all planes. This
Re: [Intel-gfx] [PATCH] drm/i915: Disable LP3 watermarks on all SNB machines
Quoting Ville Syrjälä (2018-11-13 18:40:20) > On Tue, Nov 13, 2018 at 06:25:47PM +, Chris Wilson wrote: > > Quoting Ville Syrjala (2018-11-13 18:10:23) > > > From: Ville Syrjälä > > > > > > I have a Thinkpad X220 Tablet in my hands that is losing vblank > > > interrupts whenever LP3 watermarks are used. > > > > > > If I nudge the latency value written to the WM3 register just > > > by one in either direction the problem disappears. That to me > > > suggests that the punit will not enter the corrsponding > > > powersave mode (MPLL shutdown IIRC) unless the latency value > > > in the register matches exactly what we read from SSKPD. Ie. > > > it's not really a latency value but rather just a cookie > > > by which the punit can identify the desired power saving state. > > > On HSW/BDW this was changed such that we actually just write > > > the WM level number into those bits, which makes much more > > > sense given the observed behaviour. > > > > > > We could try to handle this by disallowing LP3 watermarks > > > only when vblank interrupts are enabled but we'd first have > > > to prove that only vblank interrupts are affected, which > > > seems unlikely. Also we can't grab the wm mutex from the > > > vblank enable/disable hooks because those are called with > > > various spinlocks held. Thus we'd have to redesigne the > > > watermark locking. So to play it safe and keep the code > > > simple we simply disable LP3 watermarks on all SNB machines. > > > > > > To do that we simply zero out the latency values for > > > watermark level 3, and we adjust the watermark computation > > > to check for that. The behaviour now matches that of the > > > g4x/vlv/skl wm code in the presence of a zeroed latency > > > value. > > > > > > Cc: sta...@vger.kernel.org > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101269 > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103713 > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/i915/intel_pm.c | 41 - > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index 27498ded4949..ef1ae2ede379 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -2493,6 +2493,9 @@ static uint32_t ilk_compute_pri_wm(const struct > > > intel_crtc_state *cstate, > > > uint32_t method1, method2; > > > int cpp; > > > > > > + if (mem_value == 0) > > > + return USHRT_MAX; > > > + > > > if (!intel_wm_plane_visible(cstate, pstate)) > > > return 0; > > > > > > @@ -2522,6 +2525,9 @@ static uint32_t ilk_compute_spr_wm(const struct > > > intel_crtc_state *cstate, > > > uint32_t method1, method2; > > > int cpp; > > > > > > + if (mem_value == 0) > > > + return USHRT_MAX; > > > + > > > if (!intel_wm_plane_visible(cstate, pstate)) > > > return 0; > > > > > > @@ -2545,6 +2551,9 @@ static uint32_t ilk_compute_cur_wm(const struct > > > intel_crtc_state *cstate, > > > { > > > int cpp; > > > > > > + if (mem_value == 0) > > > + return USHRT_MAX; > > > + > > > if (!intel_wm_plane_visible(cstate, pstate)) > > > return 0; > > > > > > @@ -3008,6 +3017,34 @@ static void snb_wm_latency_quirk(struct > > > drm_i915_private *dev_priv) > > > intel_print_wm_latency(dev_priv, "Cursor", > > > dev_priv->wm.cur_latency); > > > } > > > > > > +static void snb_wm_lp3_irq_quirk(struct drm_i915_private *dev_priv) > > > +{ > > > + /* > > > +* On some SNB machines (Thinkpad X220 Tablet at least) > > > +* LP3 usage can cause vblank interrupts to be lost. > > > +* The DEIIR bit will go high but it looks like the CPU > > > +* never gets interrupted. > > > +* > > > +* It's not clear whether other interrupt source could > > > +* be affected or if this is somehow limited to vblank > > > +* interrupts only. To play it safe we disable LP3 > > > +* watermarks entirely. > > > +*/ > > > + if (dev_priv->wm.pri_latency[3] == 0 && > > > + dev_priv->wm.spr_latency[3] == 0 && > > > + dev_priv->wm.cur_latency[3] == 0) > > > + return; > > > + > > > + dev_priv->wm.pri_latency[3] = 0; > > > + dev_priv->wm.spr_latency[3] = 0; > > > + dev_priv->wm.cur_latency[3] = 0; > > > > -> return USHRT_MAX for pri/spr/cur planes. > > -> result->enable = false; > > > > Only question then why USHRT_MAX for a u32 parameter? Seems to be copied > > from gm45 where it is a u16 parameter instead. > > A good question. My excuse is that I was expecting it to be a u16. > The max value the registers can hold is 11 bits, so u16 would be > more than enough for our needs. > > Looks like we store these as u32 in the
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Disable LP3 watermarks on all SNB machines
== Series Details == Series: drm/i915: Disable LP3 watermarks on all SNB machines URL : https://patchwork.freedesktop.org/series/52440/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5133_full -> Patchwork_10817_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10817_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10817_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_10817_full: === IGT changes === Warnings igt@kms_atomic_interruptible@universal-setplane-cursor: shard-snb: SKIP -> PASS +2 == Known issues == Here are the changes found in Patchwork_10817_full that come from known issues: === IGT changes === Issues hit igt@drv_suspend@fence-restore-tiled2untiled: shard-kbl: NOTRUN -> INCOMPLETE (fdo#103665) igt@gem_cpu_reloc@full: shard-skl: PASS -> INCOMPLETE (fdo#108073) igt@gem_userptr_blits@readonly-unsync: shard-skl: NOTRUN -> INCOMPLETE (fdo#108074) igt@kms_cursor_legacy@cursorb-vs-flipb-toggle: shard-glk: PASS -> DMESG-WARN (fdo#105763, fdo#106538) igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled: shard-skl: PASS -> FAIL (fdo#103184) igt@kms_flip@2x-flip-vs-modeset-interruptible: shard-hsw: PASS -> DMESG-WARN (fdo#102614) igt@kms_flip@flip-vs-expired-vblank-interruptible: shard-skl: PASS -> FAIL (fdo#105363) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite: shard-apl: PASS -> FAIL (fdo#103167) +1 igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen: shard-glk: PASS -> FAIL (fdo#103167) +1 igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb: shard-skl: NOTRUN -> FAIL (fdo#108145) igt@kms_plane_multiple@atomic-pipe-a-tiling-x: shard-apl: PASS -> FAIL (fdo#103166) igt@kms_plane_multiple@atomic-pipe-c-tiling-y: shard-glk: PASS -> FAIL (fdo#103166) igt@pm_rpm@dpms-mode-unset-non-lpsp: shard-skl: SKIP -> INCOMPLETE (fdo#107807) +1 igt@pm_rpm@drm-resources-equal: shard-skl: PASS -> INCOMPLETE (fdo#107807) igt@pm_rpm@gem-execbuf-stress-extra-wait: shard-skl: PASS -> INCOMPLETE (fdo#107803, fdo#107807) Possible fixes igt@drv_suspend@forcewake: shard-kbl: INCOMPLETE (fdo#103665) -> PASS igt@drv_suspend@shrink: shard-skl: INCOMPLETE (fdo#106886) -> PASS shard-glk: INCOMPLETE (fdo#103359, fdo#106886, k.org#198133) -> PASS igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b: shard-kbl: DMESG-WARN (fdo#107956) -> PASS igt@kms_ccs@pipe-a-crc-sprite-planes-basic: shard-glk: FAIL (fdo#108145) -> PASS igt@kms_flip@plain-flip-fb-recreate-interruptible: shard-skl: FAIL (fdo#100368) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu: shard-apl: FAIL (fdo#103167) -> PASS +1 igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-onoff: shard-glk: FAIL (fdo#103167) -> PASS +2 igt@kms_plane@pixel-format-pipe-b-planes: shard-apl: FAIL (fdo#103166) -> PASS igt@kms_setmode@basic: shard-apl: FAIL (fdo#99912) -> PASS fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614 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#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359 fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363 fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763 fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538 fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886 fdo#107803 https://bugs.freedesktop.org/show_bug.cgi?id=107803 fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807 fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956 fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073 fdo#108074 https://bugs.freedesktop.org/show_bug.cgi?id=108074 fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145 fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133 == Participating hosts (6 -> 6) ==
[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: Use explicit old crtc state in skl_compute_wm()
== Series Details == Series: series starting with [1/3] drm/i915: Use explicit old crtc state in skl_compute_wm() URL : https://patchwork.freedesktop.org/series/52436/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5133_full -> Patchwork_10816_full = == Summary - WARNING == Minor unknown changes coming with Patchwork_10816_full need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10816_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_10816_full: === IGT changes === Warnings igt@kms_atomic_interruptible@universal-setplane-cursor: shard-snb: SKIP -> PASS +2 igt@pm_rc6_residency@rc6-accuracy: shard-kbl: SKIP -> PASS +1 == Known issues == Here are the changes found in Patchwork_10816_full that come from known issues: === IGT changes === Issues hit igt@gem_ppgtt@blt-vs-render-ctx0: shard-kbl: PASS -> INCOMPLETE (fdo#106023, fdo#103665, fdo#106887) igt@gem_userptr_blits@readonly-unsync: shard-skl: NOTRUN -> INCOMPLETE (fdo#108074) igt@kms_ccs@pipe-b-crc-sprite-planes-basic: shard-glk: PASS -> FAIL (fdo#108145) igt@kms_chv_cursor_fail@pipe-b-64x64-right-edge: shard-glk: PASS -> DMESG-FAIL (fdo#106538, fdo#104671) igt@kms_cursor_crc@cursor-256x256-suspend: shard-apl: PASS -> FAIL (fdo#103191, fdo#103232) igt@kms_cursor_legacy@cursorb-vs-flipb-toggle: shard-glk: PASS -> DMESG-WARN (fdo#105763, fdo#106538) igt@kms_flip@2x-flip-vs-expired-vblank: shard-glk: PASS -> FAIL (fdo#105363) igt@kms_flip@modeset-vs-vblank-race-interruptible: shard-snb: PASS -> FAIL (fdo#103060) igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff: shard-apl: PASS -> FAIL (fdo#103167) +1 igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-gtt: shard-glk: PASS -> FAIL (fdo#103167) igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-render: shard-glk: PASS -> DMESG-FAIL (fdo#106538) igt@kms_plane@plane-panning-bottom-right-pipe-b-planes: shard-skl: PASS -> FAIL (fdo#103166) igt@kms_plane@plane-position-covered-pipe-b-planes: shard-glk: PASS -> FAIL (fdo#103166) +1 igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max: shard-skl: NOTRUN -> FAIL (fdo#108145) igt@kms_vblank@pipe-c-accuracy-idle: shard-skl: PASS -> FAIL (fdo#102583) igt@pm_rpm@gem-execbuf: shard-skl: PASS -> INCOMPLETE (fdo#107803, fdo#107807) Possible fixes igt@drv_suspend@shrink: shard-glk: INCOMPLETE (fdo#106886, fdo#103359, k.org#198133) -> PASS igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b: shard-glk: DMESG-WARN (fdo#107956) -> PASS igt@kms_ccs@pipe-a-crc-sprite-planes-basic: shard-glk: FAIL (fdo#108145) -> PASS igt@kms_cursor_crc@cursor-128x128-sliding: shard-apl: FAIL (fdo#103232) -> PASS igt@kms_flip@plain-flip-fb-recreate-interruptible: shard-skl: FAIL (fdo#100368) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu: shard-apl: FAIL (fdo#103167) -> PASS igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move: shard-glk: FAIL (fdo#103167) -> PASS +2 igt@kms_plane@pixel-format-pipe-b-planes: shard-apl: FAIL (fdo#103166) -> PASS igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: shard-skl: FAIL (fdo#107815) -> PASS igt@kms_setmode@basic: shard-kbl: FAIL (fdo#99912) -> PASS igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend: shard-skl: INCOMPLETE (fdo#107773, fdo#104108) -> PASS Warnings igt@pm_backlight@fade_with_suspend: shard-skl: FAIL (fdo#107847) -> INCOMPLETE (fdo#107773) fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368 fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583 fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060 fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 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#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665 fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108 fdo#104671
Re: [Intel-gfx] [PATCH v2 2/3] drm: Add CRTC background color property (v2)
On Tue, Nov 13, 2018 at 03:21:48PM -0800, Matt Roper wrote: > Some display controllers can be programmed to present non-black colors > for pixels not covered by any plane (or pixels covered by the > transparent regions of higher planes). Compositors that want a UI with > a solid color background can potentially save memory bandwidth by > setting the CRTC background property and using smaller planes to display > the rest of the content. > > To avoid confusion between different ways of encoding RGB data, we > define a standard 64-bit format that should be used for this property's > value. Helper functions and macros are provided to generate and dissect > values in this standard format with varying component precision values. > > v2: > - Swap internal representation's blue and red bits to make it easier >to read if printed out. (Ville) > - Document bgcolor property in drm_blend.c. (Sean Paul) > - s/background_color/bgcolor/ for consistency between property name and >value storage field. (Sean Paul) > - Add a convenience function to attach property to a given crtc. > > Cc: dri-de...@lists.freedesktop.org > Cc: wei.c...@intel.com > Cc: harish.krupo@intel.com > Cc: Ville Syrjälä > Cc: Sean Paul > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/drm_atomic_state_helper.c | 1 + > drivers/gpu/drm/drm_atomic_uapi.c | 5 + > drivers/gpu/drm/drm_blend.c | 21 ++--- > drivers/gpu/drm/drm_mode_config.c | 6 ++ > include/drm/drm_blend.h | 1 + > include/drm/drm_crtc.h| 17 + > include/drm/drm_mode_config.h | 5 + > include/uapi/drm/drm_mode.h | 26 ++ > 8 files changed, 79 insertions(+), 3 deletions(-) /snip > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index d3e0fe31efc5..7c4f902aa290 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -888,6 +888,32 @@ struct drm_mode_revoke_lease { > __u32 lessee_id; > }; > > +/* > + * Put RGBA values into a standard 64-bit representation that can be used > + * for ioctl parameters, inter-driver commmunication, etc. If the component > + * values being provided contain less than 16 bits of precision, they'll > + * be shifted into the most significant bits. > + */ > +static inline __u64 > +drm_rgba(__u8 bpc, __u16 red, __u16 green, __u16 blue, __u16 alpha) > +{ > + int msb_shift = 16 - bpc; > + > + return (__u64)alpha << msb_shift << 48 | > +(__u64)red << msb_shift << 32 | > +(__u64)green << msb_shift << 16 | > +(__u64)blue << msb_shift; > +} > + > +/* > + * Extract the specified number of bits of a specific color component from a > + * standard 64-bit RGBA value. > + */ > +#define DRM_RGBA_BLUE(c, numbits) (__u16)((c & 0xull) >> > (16-numbits)) > +#define DRM_RGBA_GREEN(c, numbits) (__u16)((c & 0xull<<16) >> > (32-numbits)) > +#define DRM_RGBA_RED(c, numbits) (__u16)((c & 0xull<<32) >> > (48-numbits)) > +#define DRM_RGBA_ALPHA(c, numbits) (__u16)((c & 0xull<<48) >> > (64-numbits)) #define DRM_RGBA_COMP(c, shift, numbits) \ (__u16)(((c) & 0xull << (shift)) >> (32 - ((shift) + 16 - (numbits))) #define DRM_RGBA_BLUE(c, numbits) DRM_RGBA_COMP(c, 0, numbits) #define DRM_RGBA_GREEN(c, numbits) DRM_RGBA_COMP(c, 16, numbits) #define DRM_RGBA_RED(c, numbits) DRM_RGBA_COMP(c, 32, numbits) #define DRM_RGBA_ALPHA(c, numbits) DRM_RGBA_COMP(c, 48, numbits) With that, Reviewed-by: Sean Paul > + > #if defined(__cplusplus) > } > #endif > -- > 2.14.4 > -- Sean Paul, Software Engineer, Google / Chromium OS ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: remove excess line continuation backslashes
== Series Details == Series: drm/i915: remove excess line continuation backslashes URL : https://patchwork.freedesktop.org/series/52477/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5139 -> Patchwork_10825 = == Summary - WARNING == Minor unknown changes coming with Patchwork_10825 need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10825, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/52477/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10825: === IGT changes === Warnings igt@pm_rpm@module-reload: fi-kbl-7567u: PASS -> SKIP == Known issues == Here are the changes found in Patchwork_10825 that come from known issues: === IGT changes === Issues hit igt@drv_module_reload@basic-no-display: fi-kbl-7567u: PASS -> DMESG-WARN (fdo#105602) +2 igt@drv_selftest@live_contexts: fi-icl-u: NOTRUN -> DMESG-FAIL (fdo#108569) igt@kms_frontbuffer_tracking@basic: fi-icl-u2: PASS -> FAIL (fdo#103167) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-byt-clapper: PASS -> FAIL (fdo#107362, fdo#103191) Possible fixes igt@kms_frontbuffer_tracking@basic: fi-byt-clapper: FAIL (fdo#103167) -> PASS igt@kms_pipe_crc_basic@hang-read-crc-pipe-a: fi-byt-clapper: FAIL (fdo#107362, fdo#103191) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-icl-u: INCOMPLETE (fdo#107713) -> PASS fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713 fdo#108569 https://bugs.freedesktop.org/show_bug.cgi?id=108569 == Participating hosts (54 -> 47) == Missing(7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus == Build changes == * Linux: CI_DRM_5139 -> Patchwork_10825 CI_DRM_5139: 5552717626ee1261b76154399b002a3cde69b0a5 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10825: bc94bc0400d5d851ab509925f9ddece4cdbfe89a @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == bc94bc0400d5 drm/i915: remove excess line continuation backslashes == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10825/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: use appropriate integer types for flags
== Series Details == Series: drm/i915: use appropriate integer types for flags URL : https://patchwork.freedesktop.org/series/52481/ State : success == Summary == = CI Bug Log - changes from CI_DRM_5138 -> Patchwork_10824 = == Summary - SUCCESS == No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/52481/revisions/1/mbox/ == Known issues == Here are the changes found in Patchwork_10824 that come from known issues: === IGT changes === Issues hit igt@drv_selftest@live_hangcheck: fi-kbl-7560u: PASS -> INCOMPLETE (fdo#108044) igt@drv_selftest@live_objects: fi-cfl-8109u: PASS -> INCOMPLETE (fdo#108474) igt@gem_exec_suspend@basic-s3: fi-cfl-8109u: PASS -> DMESG-WARN (k.org#200587, fdo#106612) fi-icl-u2: PASS -> DMESG-WARN (fdo#107724) igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b: fi-byt-clapper: PASS -> FAIL (fdo#103191, fdo#107362) Possible fixes igt@drv_selftest@live_contexts: fi-bsw-n3050: DMESG-FAIL (fdo#108656, fdo#108626) -> PASS igt@drv_selftest@live_hangcheck: fi-skl-6700k2: INCOMPLETE -> PASS igt@gem_ctx_create@basic-files: fi-bsw-n3050: FAIL (fdo#108656) -> PASS igt@kms_pipe_crc_basic@hang-read-crc-pipe-b: fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-skl-6600u: INCOMPLETE (fdo#104108) -> PASS fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108 fdo#106612 https://bugs.freedesktop.org/show_bug.cgi?id=106612 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724 fdo#108044 https://bugs.freedesktop.org/show_bug.cgi?id=108044 fdo#108474 https://bugs.freedesktop.org/show_bug.cgi?id=108474 fdo#108626 https://bugs.freedesktop.org/show_bug.cgi?id=108626 fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656 k.org#200587 https://bugzilla.kernel.org/show_bug.cgi?id=200587 == Participating hosts (52 -> 46) == Additional (1): fi-pnv-d510 Missing(7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u == Build changes == * Linux: CI_DRM_5138 -> Patchwork_10824 CI_DRM_5138: 60c8a0cfdd1e3f7eb3e093a39f231e5d8aa507e3 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10824: 04bef9eefcf094d2a1693b711ec91c3ac3942746 @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 04bef9eefcf0 drm/i915: use appropriate integer types for flags == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10824/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: remove excess line continuation backslashes
== Series Details == Series: drm/i915: remove excess line continuation backslashes URL : https://patchwork.freedesktop.org/series/52477/ State : failure == Summary == = CI Bug Log - changes from CI_DRM_5138 -> Patchwork_10823 = == Summary - FAILURE == Serious unknown changes coming with Patchwork_10823 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_10823, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/52477/revisions/1/mbox/ == Possible new issues == Here are the unknown changes that may have been introduced in Patchwork_10823: === IGT changes === Possible regressions igt@drv_selftest@live_contexts: fi-kbl-7560u: PASS -> INCOMPLETE == Known issues == Here are the changes found in Patchwork_10823 that come from known issues: === IGT changes === Issues hit igt@kms_flip@basic-flip-vs-modeset: fi-hsw-4770r: PASS -> DMESG-WARN (fdo#105602) Possible fixes igt@drv_selftest@live_contexts: fi-bsw-n3050: DMESG-FAIL (fdo#108656, fdo#108626) -> PASS igt@drv_selftest@live_hangcheck: fi-skl-6700k2: INCOMPLETE -> PASS igt@gem_ctx_create@basic-files: fi-bsw-n3050: FAIL (fdo#108656) -> PASS igt@kms_chamelium@common-hpd-after-suspend: fi-skl-6700k2: FAIL (fdo#103841) -> PASS igt@kms_pipe_crc_basic@hang-read-crc-pipe-b: fi-byt-clapper: FAIL (fdo#103191, fdo#107362) -> PASS igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: fi-skl-6600u: INCOMPLETE (fdo#104108) -> PASS fi-blb-e6850: INCOMPLETE (fdo#107718) -> PASS fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191 fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841 fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108 fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602 fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362 fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718 fdo#108626 https://bugs.freedesktop.org/show_bug.cgi?id=108626 fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656 == Participating hosts (52 -> 44) == Missing(8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-glk-dsi fi-icl-u2 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 == Build changes == * Linux: CI_DRM_5138 -> Patchwork_10823 CI_DRM_5138: 60c8a0cfdd1e3f7eb3e093a39f231e5d8aa507e3 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_4714: cab148ca3ec904a94d0cd43476cf7e1f8663f906 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_10823: c30300b64541c9715ac7f83312d805ca0c03e43b @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == c30300b64541 drm/i915: remove excess line continuation backslashes == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10823/issues.html ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v5 2/8] drm: Add Plane Degamma properties
>-Original Message- >From: Roper, Matthew D >Sent: Wednesday, November 14, 2018 6:39 AM >To: Shankar, Uma >Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; >dcasta...@chromium.org; alexandru-cosmin.gheor...@arm.com; >seanp...@chromium.org; Syrjala, Ville ; >harry.wentl...@amd.com; Lankhorst, Maarten >Subject: Re: [Intel-gfx] [RFC v5 2/8] drm: Add Plane Degamma properties > >On Sun, Sep 16, 2018 at 01:45:25PM +0530, Uma Shankar wrote: >> Add Plane Degamma as a blob property and plane degamma size as a range >> property. >> >> v2: Rebase >> >> v3: Fixed Sean, Paul's review comments. Moved the property from >> mode_config to drm_plane. Created a helper function to instantiate >> these properties and removed from drm_mode_create_standard_properties >> Added property documentation as suggested by Daniel, Vetter. >> >> v4: Rebase >> >> v5: Added "Display Color Hardware Pipeline" flow to kernel >> documentation as suggested by "Ville Syrjala" and "Brian Starkey". >> Moved the property creation to drm_color_mgmt.c file to consolidate >> all color operations at one place. >> >> Signed-off-by: Uma Shankar >> Reviewed-by: Alexandru Gheorghe >> --- >> Documentation/gpu/drm-kms.rst | 90 >+ >> drivers/gpu/drm/drm_atomic.c| 13 ++ >> drivers/gpu/drm/drm_atomic_helper.c | 6 +++ >> drivers/gpu/drm/drm_color_mgmt.c| 44 -- >> include/drm/drm_plane.h | 24 ++ >> 5 files changed, 174 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/gpu/drm-kms.rst >> b/Documentation/gpu/drm-kms.rst index f8f5bf1..253d546 100644 >> --- a/Documentation/gpu/drm-kms.rst >> +++ b/Documentation/gpu/drm-kms.rst >> @@ -551,12 +551,102 @@ Plane Composition Properties Color Management >> Properties >> --- >> >> +Below is how a typical hardware pipeline for color will look like: >> + >> +.. kernel-render:: DOT >> + :alt: Display Color Pipeline >> + :caption: Display Color Pipeline Overview >> + >> + digraph "KMS" { >> + node [shape=box] >> + >> + subgraph cluster_static { >> + style=dashed >> + label="Display Color Hardware Blocks" >> + >> + node [bgcolor=grey style=filled] >> + "Plane Degamma A" -> "Plane CSC/CTM A" >> + "Plane CSC/CTM A" -> "Plane Gamma A" >> + "Pipe Blender" [color=lightblue,style=filled, width=5.25, >> height=0.75]; >> + "Plane Gamma A" -> "Pipe Blender" >> + "Pipe Blender" -> "Pipe DeGamma" >> + "Pipe DeGamma" -> "Pipe CSC/CTM" >> + "Pipe CSC/CTM" -> "Pipe Gamma" >> + "Pipe Gamma" -> "Pipe Output" >> + } >> + >> + subgraph cluster_static { >> + style=dashed >> + >> + node [shape=box] >> + "Plane Degamma B" -> "Plane CSC/CTM B" >> + "Plane CSC/CTM B" -> "Plane Gamma B" >> + "Plane Gamma B" -> "Pipe Blender" >> + } >> + >> + subgraph cluster_static { >> + style=dashed >> + >> + node [shape=box] >> + "Plane Degamma C" -> "Plane CSC/CTM C" >> + "Plane CSC/CTM C" -> "Plane Gamma C" >> + "Plane Gamma C" -> "Pipe Blender" >> + } >> + >> + subgraph cluster_fb { >> + style=dashed >> + label="RAM" >> + >> + node [shape=box width=1.7 height=0.2] >> + >> + "FB 1" -> "Plane Degamma A" >> + "FB 2" -> "Plane Degamma B" >> + "FB 3" -> "Plane Degamma C" >> + } >> + } >> + >> +In real world usecases, >> + >> +1. Plane Degamma can be used to linearize a non linear gamma encoded >> +framebuffer. This is needed to do any linear math like color space >> +conversion. For ex, linearize frames encoded in SRGB or by HDR curve. >> + >> +2. Later Plane CTM block can convert the content to some different >> +colorspace. For ex, SRGB to BT2020 etc. >> + >> +3. Plane Gamma block can be used later to re-apply the non-linear >> +curve. This can also be used to apply Tone Mapping for HDR usecases. >> + >> +All the layers or framebuffers need to be converted to same color >> +space and format before blending. The plane color hardware blocks can >> +help with this. Once the Data is blended, similar color processing >> +can be done on blended output using pipe color hardware blocks. >> + >> +DRM Properties have been created to define and expose all these >> +hardware blocks to userspace. A userspace application (compositor or >> +any color app) can use these interfaces and define policies to >> +efficiently use the display hardware for such color operations. >> + >> +Pipe Color Management Properties >> +- >> + >> .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c >> :doc: overview >> >> .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c >> :export: >> >> +Plane Color Management Properties >> +- >> + >> +.. kernel-doc::
[Intel-gfx] [PATCH v2 -fixes 3/3] drm/i915: Account for scale factor when calculating initial phase
From: Ville Syrjälä To get the initial phase correct we need to account for the scale factor as well. I forgot this initially and was mostly looking at heavily upscaled content where the minor difference between -0.5 and the proper initial phase was not readily apparent. And let's toss in a comment that tries to explain the formula a little bit. v2: The initial phase upper limit is 1.5, not 24.0! Cc: Maarten Lankhorst Fixes: 0a59952b24e2 ("drm/i915: Configure SKL+ scaler initial phase correctly") Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181029181820.21956-1-ville.syrj...@linux.intel.com Tested-by: Juha-Pekka Heikkila Tested-by: Maarten Lankhorst #irc Reviewed-by: Maarten Lankhorst #irc (cherry picked from commit e7a278a329dd8aa2c70c564849f164cb5673689c) --- drivers/gpu/drm/i915/intel_display.c | 45 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_sprite.c | 20 + 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 23d8008a93bb..a54843fdeb2f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4850,8 +4850,31 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe) * chroma samples for both of the luma samples, and thus we don't * actually get the expected MPEG2 chroma siting convention :( * The same behaviour is observed on pre-SKL platforms as well. + * + * Theory behind the formula (note that we ignore sub-pixel + * source coordinates): + * s = source sample position + * d = destination sample position + * + * Downscaling 4:1: + * -0.5 + * | 0.0 + * | | 1.5 (initial phase) + * | | | + * v v v + * | s | s | s | s | + * | d | + * + * Upscaling 1:4: + * -0.5 + * | -0.375 (initial phase) + * | | 0.0 + * | | | + * v v v + * | s | + * | d | d | d | d | */ -u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_cosited) { int phase = -0x8000; u16 trip = 0; @@ -4859,6 +4882,15 @@ u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) if (chroma_cosited) phase += (sub - 1) * 0x8000 / sub; + phase += scale / (2 * sub); + + /* +* Hardware initial phase limited to [-0.5:1.5]. +* Since the max hardware scale factor is 3.0, we +* should never actually excdeed 1.0 here. +*/ + WARN_ON(phase < -0x8000 || phase > 0x18000); + if (phase < 0) phase = 0x1 + phase; else @@ -5067,13 +5099,20 @@ static void skylake_pfit_enable(struct intel_crtc *crtc) if (crtc->config->pch_pfit.enabled) { u16 uv_rgb_hphase, uv_rgb_vphase; + int pfit_w, pfit_h, hscale, vscale; int id; if (WARN_ON(crtc->config->scaler_state.scaler_id < 0)) return; - uv_rgb_hphase = skl_scaler_calc_phase(1, false); - uv_rgb_vphase = skl_scaler_calc_phase(1, false); + pfit_w = (crtc->config->pch_pfit.size >> 16) & 0x; + pfit_h = crtc->config->pch_pfit.size & 0x; + + hscale = (crtc->config->pipe_src_w << 16) / pfit_w; + vscale = (crtc->config->pipe_src_h << 16) / pfit_h; + + uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false); + uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false); id = scaler_state->scaler_id; I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN | diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f8dc84b2d2d3..8b298e5f012d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1646,7 +1646,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state); -u16 skl_scaler_calc_phase(int sub, bool chroma_center); +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center); int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state); int skl_max_scale(const struct intel_crtc_state *crtc_state, u32 pixel_format); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index fa7eaace5f92..d3090a7537bb 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -318,22 +318,30 @@ skl_program_scaler(struct intel_plane *plane, uint32_t crtc_h = drm_rect_height(_state->base.dst); u16 y_hphase, uv_rgb_hphase; u16 y_vphase, uv_rgb_vphase; + int hscale, vscale; + + hscale = drm_rect_calc_hscale(_state->base.src, + _state->base.dst,
Re: [Intel-gfx] [PATCH -fixes 3/3] drm/i915: Account for scale factor when calculating initial phase
On Wed, Nov 14, 2018 at 01:49:25PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > To get the initial phase correct we need to account for the scale > factor as well. I forgot this initially and was mostly looking at > heavily upscaled content where the minor difference between -0.5 > and the proper initial phase was not readily apparent. > > And let's toss in a comment that tries to explain the formula > a little bit. > > v2: The initial phase upper limit is 1.5, not 24.0! > > Cc: Maarten Lankhorst > Fixes: 0a59952b24e2 ("drm/i915: Configure SKL+ scaler initial phase > correctly") > Signed-off-by: Ville Syrjälä > Link: > https://patchwork.freedesktop.org/patch/msgid/20181029181820.21956-1-ville.syrj...@linux.intel.com > Tested-by: Juha-Pekka Heikkila > Tested-by: Maarten Lankhorst #irc > Reviewed-by: Maarten Lankhorst #irc > (cherry picked from commit e7a278a329dd8aa2c70c564849f164cb5673689c) > --- > drivers/gpu/drm/i915/intel_display.c | 45 ++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > drivers/gpu/drm/i915/intel_sprite.c | 20 + > 3 files changed, 57 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 23d8008a93bb..636738c04eb2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4850,8 +4850,31 @@ static void cpt_verify_modeset(struct drm_device *dev, > int pipe) > * chroma samples for both of the luma samples, and thus we don't > * actually get the expected MPEG2 chroma siting convention :( > * The same behaviour is observed on pre-SKL platforms as well. > + * > + * Theory behind the formula (note that we ignore sub-pixel > + * source coordinates): > + * s = source sample position > + * d = destination sample position > + * > + * Downscaling 4:1: > + * -0.5 > + * | 0.0 > + * | | 1.5 (initial phase) > + * | | | > + * v v v > + * | s | s | s | s | > + * | d | > + * > + * Upscaling 1:4: > + * -0.5 > + * | -0.375 (initial phase) > + * | | 0.0 > + * | | | > + * v v v > + * | s | > + * | d | d | d | d | > */ > -u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) > +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_cosited) > { > int phase = -0x8000; > u16 trip = 0; > @@ -4859,6 +4882,15 @@ u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) > if (chroma_cosited) > phase += (sub - 1) * 0x8000 / sub; > > + phase += scale / (2 * sub); > + > + /* > + * Hardware initial phase limited to [-0.5:1.5]. > + * Since the max hardware scale factor is 3.0, we > + * should never actually excdeed 1.0 here. > + */ > + WARN_ON(phase < -0x8000 || phase > 0x18000); > + > if (phase < 0) > phase = 0x1 + phase; > else > @@ -5067,13 +5099,20 @@ static void skylake_pfit_enable(struct intel_crtc > *crtc) > > if (crtc->config->pch_pfit.enabled) { > u16 uv_rgb_hphase, uv_rgb_vphase; > + int pfit_w, pfit_h, hscale, vscale; > int id; > > if (WARN_ON(crtc->config->scaler_state.scaler_id < 0)) > return; > > - uv_rgb_hphase = skl_scaler_calc_phase(1, false); > - uv_rgb_vphase = skl_scaler_calc_phase(1, false); > + pfit_w = (crtc_state->pch_pfit.size >> 16) & 0x; > + pfit_h = crtc_state->pch_pfit.size & 0x; > + > + hscale = (crtc_state->pipe_src_w << 16) / pfit_w; > + vscale = (crtc_state->pipe_src_h << 16) / pfit_h; Bah. This doesn't build. I'll fix and resend, and make doubly sure to build test this time. Sorry for the noise. > + > + uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false); > + uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false); > > id = scaler_state->scaler_id; > I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN | > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index f8dc84b2d2d3..8b298e5f012d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1646,7 +1646,7 @@ void intel_mode_from_pipe_config(struct > drm_display_mode *mode, > void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, > struct intel_crtc_state *crtc_state); > > -u16 skl_scaler_calc_phase(int sub, bool chroma_center); > +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center); > int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state); > int skl_max_scale(const struct intel_crtc_state *crtc_state, > u32 pixel_format); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c > b/drivers/gpu/drm/i915/intel_sprite.c > index fa7eaace5f92..d3090a7537bb 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++
Re: [Intel-gfx] [PATCH] drm/i915: use appropriate integer types for flags
On Wed, Nov 14, 2018 at 12:08:06PM +, Lionel Landwerlin wrote: > We've been dealing a number of 32/64 bits flags issues lately : > > - 085603287452fc ("drm/i915: Compare user's 64b GTT offset even on 32b") > - c58281056a8b26 ("drm/i915: Mark up GTT sizes as u64") > - 83b466b1dc5f0b ("drm/i915: Mark pin flags as u64") > > As userspace and in particular Mesa pulls in the uAPI headers and > builds up flags using the uAPI defines we should probably make those > more explicitly 32/64bits aware. > > Signed-off-by: Lionel Landwerlin > --- > include/uapi/drm/i915_drm.h | 90 ++--- > 1 file changed, 45 insertions(+), 45 deletions(-) > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index e477ef8c644e..f562c4239bd8 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -895,12 +895,12 @@ struct drm_i915_gem_exec_object2 { >*/ > __u64 offset; > > -#define EXEC_OBJECT_NEEDS_FENCE (1<<0) > -#define EXEC_OBJECT_NEEDS_GTT (1<<1) > -#define EXEC_OBJECT_WRITE (1<<2) > -#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) > -#define EXEC_OBJECT_PINNED(1<<4) > -#define EXEC_OBJECT_PAD_TO_SIZE (1<<5) > +#define EXEC_OBJECT_NEEDS_FENCE (1ULL<<0) > +#define EXEC_OBJECT_NEEDS_GTT (1ULL<<1) > +#define EXEC_OBJECT_WRITE (1ULL<<2) > +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1ULL<<3) > +#define EXEC_OBJECT_PINNED(1ULL<<4) > +#define EXEC_OBJECT_PAD_TO_SIZE (1ULL<<5) > /* The kernel implicitly tracks GPU activity on all GEM objects, and > * synchronises operations with outstanding rendering. This includes > * rendering on other devices if exported via dma-buf. However, sometimes > @@ -921,14 +921,14 @@ struct drm_i915_gem_exec_object2 { > * explicit tracking to avoid rendering corruption. See, for example, > * I915_PARAM_HAS_EXEC_FENCE to order execbufs and execute them > asynchronously. > */ > -#define EXEC_OBJECT_ASYNC(1<<6) > +#define EXEC_OBJECT_ASYNC(1ULL<<6) > /* Request that the contents of this execobject be copied into the error > * state upon a GPU hang involving this batch for post-mortem debugging. > * These buffers are recorded in no particular order as "user" in > * /sys/class/drm/cardN/error. Query I915_PARAM_HAS_EXEC_CAPTURE to see > * if the kernel supports this flag. > */ > -#define EXEC_OBJECT_CAPTURE (1<<7) > +#define EXEC_OBJECT_CAPTURE (1ULL<<7) > /* All remaining bits are MBZ and RESERVED FOR FUTURE USE */ > #define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_CAPTURE<<1) > __u64 flags; > @@ -946,8 +946,8 @@ struct drm_i915_gem_exec_fence { >*/ > __u32 handle; > > -#define I915_EXEC_FENCE_WAIT(1<<0) > -#define I915_EXEC_FENCE_SIGNAL (1<<1) > +#define I915_EXEC_FENCE_WAIT(1UL<<0) > +#define I915_EXEC_FENCE_SIGNAL (1UL<<1) UL doesn't make much sense to me. It can be 32 or 64 bits depending on the architecture. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: use appropriate integer types for flags
== Series Details == Series: drm/i915: use appropriate integer types for flags URL : https://patchwork.freedesktop.org/series/52481/ State : warning == Summary == $ dim checkpatch origin/drm-tip 04bef9eefcf0 drm/i915: use appropriate integer types for flags -:8: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 'commit 085603287452 ("drm/i915: Compare user's 64b GTT offset even on 32b")' #8: - 085603287452fc ("drm/i915: Compare user's 64b GTT offset even on 32b") -:9: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 'commit c58281056a8b ("drm/i915: Mark up GTT sizes as u64")' #9: - c58281056a8b26 ("drm/i915: Mark up GTT sizes as u64") -:10: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("")' - ie: 'commit 83b466b1dc5f ("drm/i915: Mark pin flags as u64")' #10: - 83b466b1dc5f0b ("drm/i915: Mark pin flags as u64") -:32: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #32: FILE: include/uapi/drm/i915_drm.h:898: +#define EXEC_OBJECT_NEEDS_FENCE (1ULL<<0) ^ -:33: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #33: FILE: include/uapi/drm/i915_drm.h:899: +#define EXEC_OBJECT_NEEDS_GTT (1ULL<<1) ^ -:34: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #34: FILE: include/uapi/drm/i915_drm.h:900: +#define EXEC_OBJECT_WRITE (1ULL<<2) ^ -:35: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #35: FILE: include/uapi/drm/i915_drm.h:901: +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1ULL<<3) ^ -:36: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #36: FILE: include/uapi/drm/i915_drm.h:902: +#define EXEC_OBJECT_PINNED (1ULL<<4) ^ -:37: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #37: FILE: include/uapi/drm/i915_drm.h:903: +#define EXEC_OBJECT_PAD_TO_SIZE (1ULL<<5) ^ -:46: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #46: FILE: include/uapi/drm/i915_drm.h:924: +#define EXEC_OBJECT_ASYNC (1ULL<<6) ^ -:54: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #54: FILE: include/uapi/drm/i915_drm.h:931: +#define EXEC_OBJECT_CAPTURE(1ULL<<7) ^ -:64: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #64: FILE: include/uapi/drm/i915_drm.h:949: +#define I915_EXEC_FENCE_WAIT(1UL<<0) ^ -:65: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #65: FILE: include/uapi/drm/i915_drm.h:950: +#define I915_EXEC_FENCE_SIGNAL (1UL<<1) ^ -:79: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #79: FILE: include/uapi/drm/i915_drm.h:975: +#define I915_EXEC_RING_MASK (7ULL<<0) ^ -:80: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #80: FILE: include/uapi/drm/i915_drm.h:976: +#define I915_EXEC_DEFAULT(0ULL<<0) ^ -:81: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #81: FILE: include/uapi/drm/i915_drm.h:977: +#define I915_EXEC_RENDER (1ULL<<0) ^ -:82: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #82: FILE: include/uapi/drm/i915_drm.h:978: +#define I915_EXEC_BSD(2ULL<<0) ^ -:83: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #83: FILE: include/uapi/drm/i915_drm.h:979: +#define I915_EXEC_BLT(3ULL<<0) ^ -:84: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #84: FILE: include/uapi/drm/i915_drm.h:980: +#define I915_EXEC_VEBOX (4ULL<<0) ^ -:96: WARNING:SPACE_BEFORE_TAB: please, no space before tabs #96: FILE: include/uapi/drm/i915_drm.h:988: +#define I915_EXEC_CONSTANTS_MASK ^I(3ULL<<6)$ -:96: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #96: FILE: include/uapi/drm/i915_drm.h:988: +#define I915_EXEC_CONSTANTS_MASK (3ULL<<6) ^ -:97: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV) #97: FILE: include/uapi/drm/i915_drm.h:989: +#define I915_EXEC_CONSTANTS_REL_GENERAL (0ULL<<6) /* default */ ^ -:98: WARNING:SPACE_BEFORE_TAB: please, no
Re: [Intel-gfx] [PATCH] drm/i915: use appropriate integer types for flags
Quoting Lionel Landwerlin (2018-11-14 12:08:06) > We've been dealing a number of 32/64 bits flags issues lately : > > - 085603287452fc ("drm/i915: Compare user's 64b GTT offset even on 32b") > - c58281056a8b26 ("drm/i915: Mark up GTT sizes as u64") > - 83b466b1dc5f0b ("drm/i915: Mark pin flags as u64") > > As userspace and in particular Mesa pulls in the uAPI headers and > builds up flags using the uAPI defines we should probably make those > more explicitly 32/64bits aware. I just want to note that since these happen to be signed values, we don't have the same issue with zero-extension of their inverses. I don't think there's user impact here; no requirement for cc:stable. > Signed-off-by: Lionel Landwerlin > --- > include/uapi/drm/i915_drm.h | 90 ++--- > 1 file changed, 45 insertions(+), 45 deletions(-) > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index e477ef8c644e..f562c4239bd8 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -895,12 +895,12 @@ struct drm_i915_gem_exec_object2 { > */ > __u64 offset; > > -#define EXEC_OBJECT_NEEDS_FENCE (1<<0) > -#define EXEC_OBJECT_NEEDS_GTT (1<<1) > -#define EXEC_OBJECT_WRITE (1<<2) > -#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) > -#define EXEC_OBJECT_PINNED (1<<4) > -#define EXEC_OBJECT_PAD_TO_SIZE (1<<5) > +#define EXEC_OBJECT_NEEDS_FENCE (1ULL<<0) We should probably appease CODING_STYLE a bit more while we are here; (1ULL << 0) -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: use appropriate integer types for flags
We've been dealing a number of 32/64 bits flags issues lately : - 085603287452fc ("drm/i915: Compare user's 64b GTT offset even on 32b") - c58281056a8b26 ("drm/i915: Mark up GTT sizes as u64") - 83b466b1dc5f0b ("drm/i915: Mark pin flags as u64") As userspace and in particular Mesa pulls in the uAPI headers and builds up flags using the uAPI defines we should probably make those more explicitly 32/64bits aware. Signed-off-by: Lionel Landwerlin --- include/uapi/drm/i915_drm.h | 90 ++--- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index e477ef8c644e..f562c4239bd8 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -895,12 +895,12 @@ struct drm_i915_gem_exec_object2 { */ __u64 offset; -#define EXEC_OBJECT_NEEDS_FENCE (1<<0) -#define EXEC_OBJECT_NEEDS_GTT (1<<1) -#define EXEC_OBJECT_WRITE (1<<2) -#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3) -#define EXEC_OBJECT_PINNED (1<<4) -#define EXEC_OBJECT_PAD_TO_SIZE (1<<5) +#define EXEC_OBJECT_NEEDS_FENCE (1ULL<<0) +#define EXEC_OBJECT_NEEDS_GTT (1ULL<<1) +#define EXEC_OBJECT_WRITE (1ULL<<2) +#define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1ULL<<3) +#define EXEC_OBJECT_PINNED (1ULL<<4) +#define EXEC_OBJECT_PAD_TO_SIZE (1ULL<<5) /* The kernel implicitly tracks GPU activity on all GEM objects, and * synchronises operations with outstanding rendering. This includes * rendering on other devices if exported via dma-buf. However, sometimes @@ -921,14 +921,14 @@ struct drm_i915_gem_exec_object2 { * explicit tracking to avoid rendering corruption. See, for example, * I915_PARAM_HAS_EXEC_FENCE to order execbufs and execute them asynchronously. */ -#define EXEC_OBJECT_ASYNC (1<<6) +#define EXEC_OBJECT_ASYNC (1ULL<<6) /* Request that the contents of this execobject be copied into the error * state upon a GPU hang involving this batch for post-mortem debugging. * These buffers are recorded in no particular order as "user" in * /sys/class/drm/cardN/error. Query I915_PARAM_HAS_EXEC_CAPTURE to see * if the kernel supports this flag. */ -#define EXEC_OBJECT_CAPTURE(1<<7) +#define EXEC_OBJECT_CAPTURE(1ULL<<7) /* All remaining bits are MBZ and RESERVED FOR FUTURE USE */ #define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_CAPTURE<<1) __u64 flags; @@ -946,8 +946,8 @@ struct drm_i915_gem_exec_fence { */ __u32 handle; -#define I915_EXEC_FENCE_WAIT(1<<0) -#define I915_EXEC_FENCE_SIGNAL (1<<1) +#define I915_EXEC_FENCE_WAIT(1UL<<0) +#define I915_EXEC_FENCE_SIGNAL (1UL<<1) #define __I915_EXEC_FENCE_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SIGNAL << 1)) __u32 flags; }; @@ -972,12 +972,12 @@ struct drm_i915_gem_execbuffer2 { * struct drm_i915_gem_exec_fence *fences. */ __u64 cliprects_ptr; -#define I915_EXEC_RING_MASK (7<<0) -#define I915_EXEC_DEFAULT(0<<0) -#define I915_EXEC_RENDER (1<<0) -#define I915_EXEC_BSD(2<<0) -#define I915_EXEC_BLT(3<<0) -#define I915_EXEC_VEBOX (4<<0) +#define I915_EXEC_RING_MASK (7ULL<<0) +#define I915_EXEC_DEFAULT(0ULL<<0) +#define I915_EXEC_RENDER (1ULL<<0) +#define I915_EXEC_BSD(2ULL<<0) +#define I915_EXEC_BLT(3ULL<<0) +#define I915_EXEC_VEBOX (4ULL<<0) /* Used for switching the constants addressing mode on gen4+ RENDER ring. * Gen6+ only supports relative addressing to dynamic state (default) and @@ -985,22 +985,22 @@ struct drm_i915_gem_execbuffer2 { * * These flags are ignored for the BSD and BLT rings. */ -#define I915_EXEC_CONSTANTS_MASK (3<<6) -#define I915_EXEC_CONSTANTS_REL_GENERAL (0<<6) /* default */ -#define I915_EXEC_CONSTANTS_ABSOLUTE (1<<6) -#define I915_EXEC_CONSTANTS_REL_SURFACE (2<<6) /* gen4/5 only */ +#define I915_EXEC_CONSTANTS_MASK (3ULL<<6) +#define I915_EXEC_CONSTANTS_REL_GENERAL (0ULL<<6) /* default */ +#define I915_EXEC_CONSTANTS_ABSOLUTE (1ULL<<6) +#define I915_EXEC_CONSTANTS_REL_SURFACE (2ULL<<6) /* gen4/5 only */ __u64 flags; __u64 rsvd1; /* now used for context info */ __u64 rsvd2; }; /** Resets the SO write offset registers for transform feedback on gen7. */ -#define I915_EXEC_GEN7_SOL_RESET (1<<8) +#define I915_EXEC_GEN7_SOL_RESET (1ULL<<8) /** Request a privileged ("secure") batch buffer. Note only available for * DRM_ROOT_ONLY | DRM_MASTER processes. */ -#define I915_EXEC_SECURE (1<<9) +#define I915_EXEC_SECURE (1ULL<<9) /** Inform the kernel that the batch is and will
Re: [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915/gen9_bc: Work around DMC bug zeroing power well requests
On Sat, Nov 10, 2018 at 12:57:13AM +, Patchwork wrote: > == Series Details == > > Series: series starting with [1/3] drm/i915/gen9_bc: Work around DMC bug > zeroing power well requests > URL : https://patchwork.freedesktop.org/series/52302/ > State : success Pushed to -dinq, thanks for the reviews. > > == Summary == > > = CI Bug Log - changes from CI_DRM_5115_full -> Patchwork_10795_full = > > == Summary - WARNING == > > Minor unknown changes coming with Patchwork_10795_full need to be verified > manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_10795_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_10795_full: > > === IGT changes === > > Warnings > > igt@kms_frontbuffer_tracking@fbc-badstride: > shard-snb: SKIP -> PASS > > igt@perf_pmu@rc6: > shard-kbl: SKIP -> PASS > > igt@pm_rc6_residency@rc6-accuracy: > shard-kbl: PASS -> SKIP > > igt@tools_test@tools_test: > shard-skl: PASS -> SKIP > > > == Known issues == > > Here are the changes found in Patchwork_10795_full that come from known > issues: > > === IGT changes === > > Issues hit > > igt@drv_suspend@forcewake: > shard-kbl: PASS -> INCOMPLETE (fdo#103665) > > igt@gem_softpin@noreloc-s3: > shard-skl: PASS -> INCOMPLETE (fdo#104108, fdo#107773) > > igt@gem_userptr_blits@readonly-unsync: > shard-skl: PASS -> INCOMPLETE (fdo#108074) > > igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c: > shard-kbl: NOTRUN -> DMESG-WARN (fdo#107956) > > igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a: > shard-apl: PASS -> DMESG-WARN (fdo#107956) > > igt@kms_chv_cursor_fail@pipe-c-256x256-top-edge: > shard-skl: PASS -> FAIL (fdo#104671) > > igt@kms_color@pipe-c-ctm-blue-to-red: > shard-skl: PASS -> FAIL (fdo#107201) > > igt@kms_color@pipe-c-degamma: > shard-apl: PASS -> FAIL (fdo#104782) > > igt@kms_cursor_crc@cursor-128x128-sliding: > shard-apl: PASS -> FAIL (fdo#103232) > > igt@kms_flip@flip-vs-expired-vblank: > shard-skl: PASS -> FAIL (fdo#105363) > > igt@kms_flip@flip-vs-modeset-vs-hang-interruptible: > shard-apl: PASS -> INCOMPLETE (fdo#103927) > > igt@kms_flip@plain-flip-ts-check-interruptible: > shard-skl: PASS -> FAIL (fdo#100368) > > igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu: > shard-apl: PASS -> FAIL (fdo#103167) > > igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen: > shard-glk: PASS -> FAIL (fdo#103167) > > igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: > shard-skl: PASS -> FAIL (fdo#107815) > > igt@kms_plane_multiple@atomic-pipe-c-tiling-y: > shard-glk: PASS -> FAIL (fdo#103166) > > igt@perf@polling: > shard-hsw: PASS -> FAIL (fdo#102252) > > igt@pm_rpm@modeset-stress-extra-wait: > shard-skl: PASS -> INCOMPLETE (fdo#107807) +2 > > igt@pm_rpm@system-suspend: > shard-skl: PASS -> INCOMPLETE (fdo#104108, fdo#107807, > fdo#107773) > > > Possible fixes > > igt@drv_suspend@shrink: > shard-glk: INCOMPLETE (fdo#103359, fdo#106886, k.org#198133) > -> PASS > > igt@gem_ctx_isolation@vecs0-s3: > shard-skl: INCOMPLETE (fdo#104108, fdo#107773) -> PASS > > igt@gem_ppgtt@blt-vs-render-ctx0: > shard-kbl: INCOMPLETE (fdo#103665, fdo#106887, fdo#106023) -> > PASS > > igt@gem_pwrite@big-gtt-fbr: > shard-apl: INCOMPLETE (fdo#103927) -> PASS > > igt@kms_busy@extended-pageflip-hang-newfb-render-c: > shard-apl: DMESG-WARN (fdo#107956) -> PASS > > igt@kms_color@pipe-c-legacy-gamma: > shard-apl: FAIL (fdo#104782) -> PASS > > igt@kms_cursor_crc@cursor-128x128-random: > shard-apl: FAIL (fdo#103232) -> PASS +2 > > igt@kms_cursor_crc@cursor-256x256-suspend: > shard-apl: FAIL (fdo#103232, fdo#103191) -> PASS > > igt@kms_cursor_legacy@cursor-vs-flip-toggle: > shard-hsw: FAIL (fdo#103355) -> PASS > > igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions: > shard-skl: FAIL (fdo#106081) -> PASS > > igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-cpu: > shard-skl: FAIL (fdo#105682) -> PASS +1 > > igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite: > shard-apl:
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Compare user's 64b GTT offset even on 32b
Reminds me that we should probably update the uAPI... struct drm_i915_gem_exec_object2 has a u64 flags and all the EXEC_OBJECT_* flags are (1< - Lionel On 25/10/2018 10:18, Chris Wilson wrote: Beware mixing unsigned long constants and 64b values, as on 32b the constant will be zero extended and discard the high 32b when used as a mask! Reported-by: Sergii Romantsov Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108282 Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/gvt/gtt.h | 1 - drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.h| 2 ++ 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h index 7a9b36176efb..bfb6f652b09f 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.h +++ b/drivers/gpu/drm/i915/gvt/gtt.h @@ -35,7 +35,6 @@ #define _GVT_GTT_H_ #define I915_GTT_PAGE_SHIFT 12 -#define I915_GTT_PAGE_MASK (~(I915_GTT_PAGE_SIZE - 1)) struct intel_vgpu_mm; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index f90a09b83370..1a1c04db6c80 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -460,7 +460,7 @@ eb_validate_vma(struct i915_execbuffer *eb, * any non-page-aligned or non-canonical addresses. */ if (unlikely(entry->flags & EXEC_OBJECT_PINNED && -entry->offset != gen8_canonical_addr(entry->offset & PAGE_MASK))) +entry->offset != gen8_canonical_addr(entry->offset & I915_GTT_PAGE_MASK))) return -EINVAL; /* pad_to_size was once a reserved field, so sanitize it */ diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index aa8307043036..5d2c5ba55ad8 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -49,6 +49,8 @@ #define I915_GTT_PAGE_SIZE I915_GTT_PAGE_SIZE_4K #define I915_GTT_MAX_PAGE_SIZE I915_GTT_PAGE_SIZE_2M +#define I915_GTT_PAGE_MASK -I915_GTT_PAGE_SIZE + #define I915_GTT_MIN_ALIGNMENT I915_GTT_PAGE_SIZE #define I915_FENCE_REG_NONE -1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH -fixes 1/3] drm/i915: Move programming plane scaler to its own function.
From: Maarten Lankhorst This cleans the code up slightly, and will make other changes easier. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/20180920102711.4184-8-maarten.lankho...@linux.intel.com (cherry picked from commit ab5c60bf76755d24ae8de5c1c6ac594934656ace) --- drivers/gpu/drm/i915/intel_sprite.c | 90 + 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 5fd2f7bf3927..873adcc20109 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -302,13 +302,63 @@ skl_plane_max_stride(struct intel_plane *plane, return min(8192 * cpp, 32768); } +static void +skl_program_scaler(struct drm_i915_private *dev_priv, + struct intel_plane *plane, + const struct intel_crtc_state *crtc_state, + const struct intel_plane_state *plane_state) +{ + enum plane_id plane_id = plane->id; + enum pipe pipe = plane->pipe; + int scaler_id = plane_state->scaler_id; + const struct intel_scaler *scaler = + _state->scaler_state.scalers[scaler_id]; + int crtc_x = plane_state->base.dst.x1; + int crtc_y = plane_state->base.dst.y1; + uint32_t crtc_w = drm_rect_width(_state->base.dst); + uint32_t crtc_h = drm_rect_height(_state->base.dst); + u16 y_hphase, uv_rgb_hphase; + u16 y_vphase, uv_rgb_vphase; + + /* Sizes are 0 based */ + crtc_w--; + crtc_h--; + + /* TODO: handle sub-pixel coordinates */ + if (plane_state->base.fb->format->format == DRM_FORMAT_NV12) { + y_hphase = skl_scaler_calc_phase(1, false); + y_vphase = skl_scaler_calc_phase(1, false); + + /* MPEG2 chroma siting convention */ + uv_rgb_hphase = skl_scaler_calc_phase(2, true); + uv_rgb_vphase = skl_scaler_calc_phase(2, false); + } else { + /* not used */ + y_hphase = 0; + y_vphase = 0; + + uv_rgb_hphase = skl_scaler_calc_phase(1, false); + uv_rgb_vphase = skl_scaler_calc_phase(1, false); + } + + I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), + PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode); + I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0); + I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id), + PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase)); + I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id), + PS_Y_PHASE(y_hphase) | PS_UV_RGB_PHASE(uv_rgb_hphase)); + I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); + I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), + ((crtc_w + 1) << 16)|(crtc_h + 1)); +} + void skl_update_plane(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); - const struct drm_framebuffer *fb = plane_state->base.fb; enum plane_id plane_id = plane->id; enum pipe pipe = plane->pipe; u32 plane_ctl = plane_state->ctl; @@ -318,8 +368,6 @@ skl_update_plane(struct intel_plane *plane, u32 aux_stride = skl_plane_stride(plane_state, 1); int crtc_x = plane_state->base.dst.x1; int crtc_y = plane_state->base.dst.y1; - uint32_t crtc_w = drm_rect_width(_state->base.dst); - uint32_t crtc_h = drm_rect_height(_state->base.dst); uint32_t x = plane_state->color_plane[0].x; uint32_t y = plane_state->color_plane[0].y; uint32_t src_w = drm_rect_width(_state->base.src) >> 16; @@ -329,8 +377,6 @@ skl_update_plane(struct intel_plane *plane, /* Sizes are 0 based */ src_w--; src_h--; - crtc_w--; - crtc_h--; spin_lock_irqsave(_priv->uncore.lock, irqflags); @@ -355,39 +401,7 @@ skl_update_plane(struct intel_plane *plane, /* program plane scaler */ if (plane_state->scaler_id >= 0) { - int scaler_id = plane_state->scaler_id; - const struct intel_scaler *scaler = - _state->scaler_state.scalers[scaler_id]; - u16 y_hphase, uv_rgb_hphase; - u16 y_vphase, uv_rgb_vphase; - - /* TODO: handle sub-pixel coordinates */ - if (fb->format->format == DRM_FORMAT_NV12) { - y_hphase = skl_scaler_calc_phase(1, false); - y_vphase = skl_scaler_calc_phase(1, false); - - /* MPEG2 chroma siting convention */ - uv_rgb_hphase = skl_scaler_calc_phase(2, true); - uv_rgb_vphase = skl_scaler_calc_phase(2, false);
[Intel-gfx] [PATCH -fixes 3/3] drm/i915: Account for scale factor when calculating initial phase
From: Ville Syrjälä To get the initial phase correct we need to account for the scale factor as well. I forgot this initially and was mostly looking at heavily upscaled content where the minor difference between -0.5 and the proper initial phase was not readily apparent. And let's toss in a comment that tries to explain the formula a little bit. v2: The initial phase upper limit is 1.5, not 24.0! Cc: Maarten Lankhorst Fixes: 0a59952b24e2 ("drm/i915: Configure SKL+ scaler initial phase correctly") Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181029181820.21956-1-ville.syrj...@linux.intel.com Tested-by: Juha-Pekka Heikkila Tested-by: Maarten Lankhorst #irc Reviewed-by: Maarten Lankhorst #irc (cherry picked from commit e7a278a329dd8aa2c70c564849f164cb5673689c) --- drivers/gpu/drm/i915/intel_display.c | 45 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_sprite.c | 20 + 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 23d8008a93bb..636738c04eb2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4850,8 +4850,31 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe) * chroma samples for both of the luma samples, and thus we don't * actually get the expected MPEG2 chroma siting convention :( * The same behaviour is observed on pre-SKL platforms as well. + * + * Theory behind the formula (note that we ignore sub-pixel + * source coordinates): + * s = source sample position + * d = destination sample position + * + * Downscaling 4:1: + * -0.5 + * | 0.0 + * | | 1.5 (initial phase) + * | | | + * v v v + * | s | s | s | s | + * | d | + * + * Upscaling 1:4: + * -0.5 + * | -0.375 (initial phase) + * | | 0.0 + * | | | + * v v v + * | s | + * | d | d | d | d | */ -u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_cosited) { int phase = -0x8000; u16 trip = 0; @@ -4859,6 +4882,15 @@ u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) if (chroma_cosited) phase += (sub - 1) * 0x8000 / sub; + phase += scale / (2 * sub); + + /* +* Hardware initial phase limited to [-0.5:1.5]. +* Since the max hardware scale factor is 3.0, we +* should never actually excdeed 1.0 here. +*/ + WARN_ON(phase < -0x8000 || phase > 0x18000); + if (phase < 0) phase = 0x1 + phase; else @@ -5067,13 +5099,20 @@ static void skylake_pfit_enable(struct intel_crtc *crtc) if (crtc->config->pch_pfit.enabled) { u16 uv_rgb_hphase, uv_rgb_vphase; + int pfit_w, pfit_h, hscale, vscale; int id; if (WARN_ON(crtc->config->scaler_state.scaler_id < 0)) return; - uv_rgb_hphase = skl_scaler_calc_phase(1, false); - uv_rgb_vphase = skl_scaler_calc_phase(1, false); + pfit_w = (crtc_state->pch_pfit.size >> 16) & 0x; + pfit_h = crtc_state->pch_pfit.size & 0x; + + hscale = (crtc_state->pipe_src_w << 16) / pfit_w; + vscale = (crtc_state->pipe_src_h << 16) / pfit_h; + + uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false); + uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false); id = scaler_state->scaler_id; I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN | diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f8dc84b2d2d3..8b298e5f012d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1646,7 +1646,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state); -u16 skl_scaler_calc_phase(int sub, bool chroma_center); +u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center); int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state); int skl_max_scale(const struct intel_crtc_state *crtc_state, u32 pixel_format); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index fa7eaace5f92..d3090a7537bb 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -318,22 +318,30 @@ skl_program_scaler(struct intel_plane *plane, uint32_t crtc_h = drm_rect_height(_state->base.dst); u16 y_hphase, uv_rgb_hphase; u16 y_vphase, uv_rgb_vphase; + int hscale, vscale; + + hscale = drm_rect_calc_hscale(_state->base.src, + _state->base.dst, +
[Intel-gfx] [PATCH -fixes 2/3] drm/i915: Clean up skl_program_scaler()
From: Ville Syrjälä Remove the "sizes are 0 based" stuff that is not even true for the scaler. v2: Rebase Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181101151736.20522-1-ville.syrj...@linux.intel.com Reviewed-by: Rodrigo Vivi (cherry picked from commit d0105af939769393d6447a04cee2d1ae12e3f09a) --- drivers/gpu/drm/i915/intel_sprite.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 873adcc20109..fa7eaace5f92 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -303,12 +303,11 @@ skl_plane_max_stride(struct intel_plane *plane, } static void -skl_program_scaler(struct drm_i915_private *dev_priv, - struct intel_plane *plane, +skl_program_scaler(struct intel_plane *plane, const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { - enum plane_id plane_id = plane->id; + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; int scaler_id = plane_state->scaler_id; const struct intel_scaler *scaler = @@ -320,10 +319,6 @@ skl_program_scaler(struct drm_i915_private *dev_priv, u16 y_hphase, uv_rgb_hphase; u16 y_vphase, uv_rgb_vphase; - /* Sizes are 0 based */ - crtc_w--; - crtc_h--; - /* TODO: handle sub-pixel coordinates */ if (plane_state->base.fb->format->format == DRM_FORMAT_NV12) { y_hphase = skl_scaler_calc_phase(1, false); @@ -342,15 +337,14 @@ skl_program_scaler(struct drm_i915_private *dev_priv, } I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id), - PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode); + PS_SCALER_EN | PS_PLANE_SEL(plane->id) | scaler->mode); I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0); I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id), PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase)); I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id), PS_Y_PHASE(y_hphase) | PS_UV_RGB_PHASE(uv_rgb_hphase)); I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y); - I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), - ((crtc_w + 1) << 16)|(crtc_h + 1)); + I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id), (crtc_w << 16) | crtc_h); } void @@ -399,9 +393,8 @@ skl_update_plane(struct intel_plane *plane, (plane_state->color_plane[1].y << 16) | plane_state->color_plane[1].x); - /* program plane scaler */ if (plane_state->scaler_id >= 0) { - skl_program_scaler(dev_priv, plane, crtc_state, plane_state); + skl_program_scaler(plane, crtc_state, plane_state); I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0); } else { -- 2.18.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: remove excess line continuation backslashes
On Wed, 14 Nov 2018, Ville Syrjälä wrote: > On Wed, Nov 14, 2018 at 01:21:30PM +0200, Jani Nikula wrote: >> While removing .palette_offsets, I removed the commas after >> .trans_offsets in the macros, but failed to remove the line continuation >> backslashes. >> >> While at it, also remove another extra comma to be in line with the >> other related macros. > > In general I like having the comma after the last element in an > array/enum/whatever. In this case it doesn't really matter, but > in cases like > > enum { > yes, > no, > + maybe, > }; > > it will result in a cleaner diff. Totally agreed; it just doesn't make a difference here, and none of the other places here have it, so off it goes. > Anyways, patch is > Reviewed-by: Ville Syrjälä Thanks. /me looks at CI expectantly. BR, Jani. > >> >> Fixes: 74c1e826427a ("drm/i915: remove palette_offsets from device info in >> favor of _PICK()") >> Cc: Ville Syrjälä >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/i915/i915_pci.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c >> b/drivers/gpu/drm/i915/i915_pci.c >> index 4ccab8372dd4..983ae7fd8217 100644 >> --- a/drivers/gpu/drm/i915/i915_pci.c >> +++ b/drivers/gpu/drm/i915/i915_pci.c >> @@ -36,13 +36,13 @@ >> .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \ >>PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \ >> .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ >> - TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } \ >> + TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } >> >> #define GEN_CHV_PIPEOFFSETS \ >> .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \ >>CHV_PIPE_C_OFFSET }, \ >> .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ >> - CHV_TRANSCODER_C_OFFSET, } \ >> + CHV_TRANSCODER_C_OFFSET } >> >> #define CURSOR_OFFSETS \ >> .cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, >> CHV_CURSOR_C_OFFSET } >> -- >> 2.11.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
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Remove special case for power well 1/MISC_IO state verification
On Mon, Nov 12, 2018 at 07:19:44PM +0200, Ville Syrjälä wrote: > On Fri, Nov 09, 2018 at 04:58:22PM +0200, Imre Deak wrote: > > Even though PW#1 and the MISC_IO power wells are managed by the > > DMC firmware (toggled dynamically if conditions allow it) from the > > driver's POV they are always on if the display core is initialized > > (always restored by DMC to the enabled state after exiting from DC5/6 > > for instance b/c of MMIO access). Accordingly we can just mark them as > > always-on and remove the special casing for them during state > > verification (thus enabling verification for these power wells too). > > > > Cc: Ramalingam C > > Cc: Daniel Vetter > > Cc: Ville Syrjälä > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 15 +++ > > 1 file changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 44a77de439f2..6b1576ae778f 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -2358,6 +2358,7 @@ static const struct i915_power_well_desc > > skl_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > First I was wondering if this somehow changes the behaviour of > __intel_display_power_is_enabled(), but it won't since all these > wells have domains==0 and so would be skipped there anyway. Yes, it should only affect the verification, otherwise doesn't change the behaviour. > > Reviewed-by: Ville Syrjälä > > > .ops = _power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2370,6 +2371,7 @@ static const struct i915_power_well_desc > > skl_power_wells[] = { > > { > > .name = "MISC IO power well", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = _power_well_ops, > > .id = SKL_DISP_PW_MISC_IO, > > @@ -2449,6 +2451,8 @@ static const struct i915_power_well_desc > > bxt_power_wells[] = { > > }, > > { > > .name = "power well 1", > > + /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = _power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2508,6 +2512,7 @@ static const struct i915_power_well_desc > > glk_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = _power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2636,6 +2641,7 @@ static const struct i915_power_well_desc > > cnl_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = _power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -2803,6 +2809,7 @@ static const struct i915_power_well_desc > > icl_power_wells[] = { > > { > > .name = "power well 1", > > /* Handled by the DMC firmware */ > > + .always_on = true, > > .domains = 0, > > .ops = _power_well_ops, > > .id = SKL_DISP_PW_1, > > @@ -3936,14 +3943,6 @@ static void intel_power_domains_verify_state(struct > > drm_i915_private *dev_priv) > > int domains_count; > > bool enabled; > > > > - /* > > -* Power wells not belonging to any domain (like the MISC_IO > > -* and PW1 power wells) are under FW control, so ignore them, > > -* since their state can change asynchronously. > > -*/ > > - if (!power_well->desc->domains) > > - continue; > > - > > enabled = power_well->desc->ops->is_enabled(dev_priv, > > power_well); > > if ((power_well->count || power_well->desc->always_on) != > > -- > > 2.13.2 > > -- > Ville Syrjälä > Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: remove excess line continuation backslashes
On Wed, Nov 14, 2018 at 01:21:30PM +0200, Jani Nikula wrote: > While removing .palette_offsets, I removed the commas after > .trans_offsets in the macros, but failed to remove the line continuation > backslashes. > > While at it, also remove another extra comma to be in line with the > other related macros. In general I like having the comma after the last element in an array/enum/whatever. In this case it doesn't really matter, but in cases like enum { yes, no, + maybe, }; it will result in a cleaner diff. Anyways, patch is Reviewed-by: Ville Syrjälä > > Fixes: 74c1e826427a ("drm/i915: remove palette_offsets from device info in > favor of _PICK()") > Cc: Ville Syrjälä > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/i915_pci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 4ccab8372dd4..983ae7fd8217 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -36,13 +36,13 @@ > .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \ > PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \ > .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ > -TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } \ > +TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } > > #define GEN_CHV_PIPEOFFSETS \ > .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \ > CHV_PIPE_C_OFFSET }, \ > .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ > -CHV_TRANSCODER_C_OFFSET, } \ > +CHV_TRANSCODER_C_OFFSET } > > #define CURSOR_OFFSETS \ > .cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, > CHV_CURSOR_C_OFFSET } > -- > 2.11.0 -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Backports for drm-intel-fixes
Quoting Joonas Lahtinen (2018-11-12 14:49:14) > The following patches with Fixes: do not cleanly apply > to drm-intel-fixes: > > - 5a3aeca97af1 ("drm/i915: Fix hpd handling for pins with two encoders") One more: - e7a278a329dd ("drm/i915: Account for scale factor when calculating initial phase") Regards, Joonas > > Please provide a backported patch ASAP, if they are relevant to be > backported. If they should not be included in drm-intel-fixes, > please confirm that by replying. > > Regards, Joonas ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: remove excess line continuation backslashes
While removing .palette_offsets, I removed the commas after .trans_offsets in the macros, but failed to remove the line continuation backslashes. While at it, also remove another extra comma to be in line with the other related macros. Fixes: 74c1e826427a ("drm/i915: remove palette_offsets from device info in favor of _PICK()") Cc: Ville Syrjälä Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 4ccab8372dd4..983ae7fd8217 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -36,13 +36,13 @@ .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \ PIPE_C_OFFSET, PIPE_EDP_OFFSET }, \ .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ - TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } \ + TRANSCODER_C_OFFSET, TRANSCODER_EDP_OFFSET } #define GEN_CHV_PIPEOFFSETS \ .pipe_offsets = { PIPE_A_OFFSET, PIPE_B_OFFSET, \ CHV_PIPE_C_OFFSET }, \ .trans_offsets = { TRANSCODER_A_OFFSET, TRANSCODER_B_OFFSET, \ - CHV_TRANSCODER_C_OFFSET, } \ + CHV_TRANSCODER_C_OFFSET } #define CURSOR_OFFSETS \ .cursor_offsets = { CURSOR_A_OFFSET, CURSOR_B_OFFSET, CHV_CURSOR_C_OFFSET } -- 2.11.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Request no slices if no active pipes
On Fri, Nov 09, 2018 at 04:44:54PM +0200, Mika Kuoppala wrote: > Skip the hardware dbuf slice update if we don't have active > pipes. With no active pipes, we don't have powerwell and thus > programming the dbuf slice counts leads to accessing > hardware without runtime pm ref. > > v2: bugzilla tag (Imre) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108654 > Cc: Imre Deak > Cc: Ville Syrjälä > Cc: Rodrigo Vivi > Signed-off-by: Mika Kuoppala > --- > drivers/gpu/drm/i915/intel_display.c| 32 +++-- > drivers/gpu/drm/i915/intel_drv.h| 3 +-- > drivers/gpu/drm/i915/intel_runtime_pm.c | 12 -- > 3 files changed, 30 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 05125c7c2aa1..0514b89611ac 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12644,23 +12644,23 @@ static void skl_update_crtcs(struct > drm_atomic_state *state) > struct intel_crtc *intel_crtc; > struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct intel_crtc_state *cstate; > - unsigned int updated = 0; > + unsigned int updated = 0, active_count = 0; > + u8 required_slices; > bool progress; > enum pipe pipe; > int i; > - u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices; > - u8 required_slices = intel_state->wm_results.ddb.enabled_slices; > - > const struct skl_ddb_entry *entries[I915_MAX_PIPES] = {}; > > - for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) > + for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > /* ignore allocations for crtc's that have been turned off. */ > - if (new_crtc_state->active) > + if (new_crtc_state->active) { > + active_count++; > entries[i] = > _intel_crtc_state(old_crtc_state)->wm.skl.ddb; > + } > + } > > - /* If 2nd DBuf slice required, enable it here */ > - if (INTEL_GEN(dev_priv) >= 11 && required_slices > hw_enabled_slices) > - icl_dbuf_slices_update(dev_priv, required_slices); Looks like this is the update that triggers the problem in the bugzilla ticket. Wondering how this can happen since we have all outputs and runtime suspended. Then by now icl_dbuf_disable() should have zeroed dev_priv->wm.skl_hw.ddb.enabled_slices and skl_compute_ddb() copied this 0 to intel_state->wm_results.ddb.enabled_slices during the compute phase of the problematic commit (since no crtcs are active this 0 should've been kept as-is). Then here both hw_enabled_slices and required_slices should be 0, but it's obviously not the case. What could happen I think is that the store in icl_dbuf_disable() / icl_dbuf_enable() can race with the load in skl_compute_ddb(). So perhaps skl_compute_ddb() copied from dev_priv enabled_slices as 1 or 2 (the only values we compute during a commit), then icl_dbuf_disable() zeroed in a racy way enabled_slices in dev_priv and we get here required_slices being 1 or 2 and hw_enabled_slices being 0. So while the change in this patch gets rid of the symptom, I think we should fix the root cause: 1. Do not update enabled_slices in icl_dbuf_enable() / icl_dbuf_disable() We instead depend on the DDB HW readout to set the correct initial value after module loading/resume and any following commit to update it if needed (based on new resolution etc.). After this we could still end up with stale values in wm.skl_hw.ddb.enabled_slices, for instance if it's 2 b/c of two pipes being enabled we wouldn't set it to 1 atm when disabling both pipes in the same atomic commit. So we'd also need the following 2 changes. 2. Force-enable only a single slice in icl_dbuf_enable() (as the spec requires) keeping the 2nd slice enabled only if BIOS has enabled it (DDB HW readout will set the correct enabled_slices afterwards). 3. Make sure we always compute the proper enabled_slices in the atomic state, by moving the calculation from intel_get_ddb_size() to earlier, performing it even if all crtcs are disabled. > + required_slices = active_count ? > intel_state->wm_results.ddb.enabled_slices : 0; > + intel_dbuf_slices_update(dev_priv, required_slices); > > /* >* Whenever the number of active pipes changes, we need to make sure we > @@ -12670,6 +12670,7 @@ static void skl_update_crtcs(struct drm_atomic_state > *state) >*/ > do { > progress = false; > + active_count = 0; > > for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, > new_crtc_state, i) { > bool vbl_wait = false; > @@ -12679,7 +12680,12 @@ static void skl_update_crtcs(struct drm_atomic_state > *state) > cstate =
Re: [Intel-gfx] [PATCH v2] Fix the possible watermark miswriting for skl+
On Wed, Nov 14, 2018 at 08:19:26AM +, Lisovskiy, Stanislav wrote: > On Tue, 2018-11-13 at 16:40 +, Chris Wilson wrote: > > Quoting Stanislav Lisovskiy (2018-11-13 14:31:38) > > > Currently whenever we attempt to recalculate > > > watermarks, we assign dirty_pipes to zero, > > > then compare current wm results to the recalculated > > > one and if they changed we set correspondent dirty_pipes > > > bit again. > > > This can lead to situation, when we are clearing dirty_pipes, > > > same wm results twice in a row and not setting dirty_pipes > > > => so that watermarks are not actually updated, which then might > > > lead to fifo underruns, crc mismatch and other issues. > > > > > > Instead, whenever we detect that wm results are changed, > > > need to set correspondent dirty_pipes bit and clear it > > > only once the change is written, but not clear it everytime > > > we attempt to recalculate those in skl_compute_wm. > > > > Ok, but are not dirty_pipes being recomputed for each commit wrt to > > the > > current HW state. Should dirty_pipes not be 0 at the start of > > compute_wm > > naturally due to it not being used before in the atomic commit > > sequence? > > -Chris > > We discussed this yesterday with Ville, as I understand the whole > drm_atomic_state is allocated each time the commit starts and it is > already naturally 0. So anyway assigning here dirty_pipes to 0 looked > redundant at least to me and Ville. > However the problem I mean was that if skl_compute_wm is invoked > twice and getting the same wm results, then we are not going to set > correspondent dirty_pipes bit and because it was zeroed in the > beginning of skl_compute_wm, we are not going to update the watermarks > at all, even though we should as it had changed. > > During our discussion with Ville, we agreed that theoretically this > shouldn't happen because skl_compute_wm(called via > compute_global_watermarks hook) should be called only once per commit > in intel_atomic_check and then results are written, when crtc is > updated and this potentially can be caught using some WARN_ON. > > What I did is I added a WARN_ON here in skl_compute_wm and removed > assigning dirty_pipes to zero(so that we can see that it might happen > that correspondent bit is already set, but we are getting here again): > > @@ -5441,9 +5441,6 @@ skl_compute_wm(struct drm_atomic_state *state) > bool changed = false; > int ret, i; > > - /* Clear all dirty flags */ > - results->dirty_pipes = 0; > - > @@ -5471,6 +5471,8 @@ skl_compute_wm(struct drm_atomic_state *state) > ret = skl_update_pipe_wm(cstate, old_pipe_wm, pipe_wm, > >>ddb, ); > if (ret) > return ret; > > + WARN_ON(!changed && (results->dirty_pipes & > drm_crtc_mask(crtc))); > + > if (changed) > results->dirty_pipes |= drm_crtc_mask(crtc); > > If skl_compute_wm is called only once we should never get it, as > we would otherwise never have correpondent crtc bit already set here > (and we would already had zeroed dirty_pipes here as we do now, the > update would be lost). > > However this WARN really catches the issue: Probably due to skl_ddb_add_affected_pipes(), which looks safe enough to me. Not sure why it sets dirty_pipes though. Either the ddb allocations change or they don't, so I don't really see why we need to mark every pipe as dirty here. They will be marked naturally by skl_ddb_add_affected_planes() anyway. So looks to me we can just drop the dirty_pipes frobbing from skl_ddb_add_affected_pipes(). > > [ 35.681909] [ cut here ] > [ 35.681917] WARN_ON(!changed && (results->dirty_pipes & > drm_crtc_mask(crtc))) > [ 35.681998] WARNING: CPU: 3 PID: 1499 at > drivers/gpu/drm/i915/intel_pm.c:5474 skl_compute_wm+0x709/0x800 [i915] > [ 35.682002] Modules linked in: cdc_ether usbnet snd_hda_codec_hdmi > snd_hda_codec_realtek snd_hda_codec_generic r8152 mii snd_hda_intel > snd_hda_codec x86_pkg_temp_thermal snd_hwdep coretemp snd_hda_core > snd_pcm pl2303 usbserial btusb btrtl btbcm btintel bluetooth > ecdh_generic mei_me mei dm_crypt crct10dif_pclmul crc32_pclmul > ghash_clmulni_intel i915 prime_numbers i2c_hid pinctrl_sunrisepoint > pinctrl_intel > [ 35.682044] CPU: 3 PID: 1499 Comm: Xorg Tainted: > GW 4.20.0-rc1-CI-CI_DRM_5076+ #123 > [ 35.682047] Hardware name: LENOVO 81A8/LNVNB161216, BIOS 5SCN34WW > 09/01/2017 > [ 35.682093] RIP: 0010:skl_compute_wm+0x709/0x800 [i915] > [ 35.682097] Code: c1 e8 11 e9 40 fe ff ff d3 e0 41 85 86 98 04 00 00 > 0f 84 85 fe ff ff 48 c7 c6 10 48 1c a0 48 c7 c7 2c 13 1b a0 e8 07 49 03 > e1 <0f> 0b 41 8b 86 98 04 00 00 e9 4f fe ff ff 44 89 ca e9 a5 f9 ff ff > [ 35.682101] RSP: 0018:c9c53a08 EFLAGS: 00010282 > [ 35.682106] RAX: RBX: 007784bf RCX: > 0006 > [ 35.682109] RDX:
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix icl workarounds whitespaces
Chris Wilson writes: > Quoting Mika Kuoppala (2018-11-09 14:53:33) >> Align icl workarounds whitespace with the rest of the file >> >> Cc: Chris Wilson >> Signed-off-by: Mika Kuoppala > > That'll do. Fine piece of Wensleydale, > Reviewed-by: Chris Wilson Both patches pushed, thanks for review. -Mika ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915/icl: Drop spurious register read from icl_dbuf_slices_update
Imre Deak writes: > On Fri, Nov 09, 2018 at 04:09:23PM +0200, Mika Kuoppala wrote: >> Register DBUF_CTL_S2 is read and it's value is not used. As >> there is no explanation why we should prime the hardware with >> read, remove it as spurious. >> >> Fixes: aa9664ffe863 ("drm/i915/icl: Enable 2nd DBuf slice only when needed") >> Cc: Mahesh Kumar >> Cc: Rodrigo Vivi >> Signed-off-by: Mika Kuoppala > > Reviewed-by: Imre Deak Pushed, thanks for review. -Mika > >> --- >> drivers/gpu/drm/i915/intel_runtime_pm.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c >> b/drivers/gpu/drm/i915/intel_runtime_pm.c >> index f945db6ea420..770de2632530 100644 >> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >> @@ -3236,8 +3236,7 @@ static u8 intel_dbuf_max_slices(struct >> drm_i915_private *dev_priv) >> void icl_dbuf_slices_update(struct drm_i915_private *dev_priv, >> u8 req_slices) >> { >> -u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices; >> -u32 val; >> +const u8 hw_enabled_slices = dev_priv->wm.skl_hw.ddb.enabled_slices; >> bool ret; >> >> if (req_slices > intel_dbuf_max_slices(dev_priv)) { >> @@ -3248,7 +3247,6 @@ void icl_dbuf_slices_update(struct drm_i915_private >> *dev_priv, >> if (req_slices == hw_enabled_slices || req_slices == 0) >> return; >> >> -val = I915_READ(DBUF_CTL_S2); >> if (req_slices > hw_enabled_slices) >> ret = intel_dbuf_slice_set(dev_priv, DBUF_CTL_S2, true); >> else >> -- >> 2.17.1 >> >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] Fix the possible watermark miswriting for skl+
On Tue, 2018-11-13 at 16:40 +, Chris Wilson wrote: > Quoting Stanislav Lisovskiy (2018-11-13 14:31:38) > > Currently whenever we attempt to recalculate > > watermarks, we assign dirty_pipes to zero, > > then compare current wm results to the recalculated > > one and if they changed we set correspondent dirty_pipes > > bit again. > > This can lead to situation, when we are clearing dirty_pipes, > > same wm results twice in a row and not setting dirty_pipes > > => so that watermarks are not actually updated, which then might > > lead to fifo underruns, crc mismatch and other issues. > > > > Instead, whenever we detect that wm results are changed, > > need to set correspondent dirty_pipes bit and clear it > > only once the change is written, but not clear it everytime > > we attempt to recalculate those in skl_compute_wm. > > Ok, but are not dirty_pipes being recomputed for each commit wrt to > the > current HW state. Should dirty_pipes not be 0 at the start of > compute_wm > naturally due to it not being used before in the atomic commit > sequence? > -Chris We discussed this yesterday with Ville, as I understand the whole drm_atomic_state is allocated each time the commit starts and it is already naturally 0. So anyway assigning here dirty_pipes to 0 looked redundant at least to me and Ville. However the problem I mean was that if skl_compute_wm is invoked twice and getting the same wm results, then we are not going to set correspondent dirty_pipes bit and because it was zeroed in the beginning of skl_compute_wm, we are not going to update the watermarks at all, even though we should as it had changed. During our discussion with Ville, we agreed that theoretically this shouldn't happen because skl_compute_wm(called via compute_global_watermarks hook) should be called only once per commit in intel_atomic_check and then results are written, when crtc is updated and this potentially can be caught using some WARN_ON. What I did is I added a WARN_ON here in skl_compute_wm and removed assigning dirty_pipes to zero(so that we can see that it might happen that correspondent bit is already set, but we are getting here again): @@ -5441,9 +5441,6 @@ skl_compute_wm(struct drm_atomic_state *state) bool changed = false; int ret, i; - /* Clear all dirty flags */ - results->dirty_pipes = 0; - @@ -5471,6 +5471,8 @@ skl_compute_wm(struct drm_atomic_state *state) ret = skl_update_pipe_wm(cstate, old_pipe_wm, pipe_wm, >ddb, ); if (ret) return ret; + WARN_ON(!changed && (results->dirty_pipes & drm_crtc_mask(crtc))); + if (changed) results->dirty_pipes |= drm_crtc_mask(crtc); If skl_compute_wm is called only once we should never get it, as we would otherwise never have correpondent crtc bit already set here (and we would already had zeroed dirty_pipes here as we do now, the update would be lost). However this WARN really catches the issue: [ 35.681909] [ cut here ] [ 35.681917] WARN_ON(!changed && (results->dirty_pipes & drm_crtc_mask(crtc))) [ 35.681998] WARNING: CPU: 3 PID: 1499 at drivers/gpu/drm/i915/intel_pm.c:5474 skl_compute_wm+0x709/0x800 [i915] [ 35.682002] Modules linked in: cdc_ether usbnet snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic r8152 mii snd_hda_intel snd_hda_codec x86_pkg_temp_thermal snd_hwdep coretemp snd_hda_core snd_pcm pl2303 usbserial btusb btrtl btbcm btintel bluetooth ecdh_generic mei_me mei dm_crypt crct10dif_pclmul crc32_pclmul ghash_clmulni_intel i915 prime_numbers i2c_hid pinctrl_sunrisepoint pinctrl_intel [ 35.682044] CPU: 3 PID: 1499 Comm: Xorg Tainted: GW 4.20.0-rc1-CI-CI_DRM_5076+ #123 [ 35.682047] Hardware name: LENOVO 81A8/LNVNB161216, BIOS 5SCN34WW 09/01/2017 [ 35.682093] RIP: 0010:skl_compute_wm+0x709/0x800 [i915] [ 35.682097] Code: c1 e8 11 e9 40 fe ff ff d3 e0 41 85 86 98 04 00 00 0f 84 85 fe ff ff 48 c7 c6 10 48 1c a0 48 c7 c7 2c 13 1b a0 e8 07 49 03 e1 <0f> 0b 41 8b 86 98 04 00 00 e9 4f fe ff ff 44 89 ca e9 a5 f9 ff ff [ 35.682101] RSP: 0018:c9c53a08 EFLAGS: 00010282 [ 35.682106] RAX: RBX: 007784bf RCX: 0006 [ 35.682109] RDX: 0006 RSI: 8212886a RDI: 820d6d57 [ 35.682112] RBP: 88029ef4 R08: 1d5e3b8b R09: [ 35.682116] R10: 88029be2ea54 R11: R12: 880299db6fc8 [ 35.682119] R13: 88029be2e678 R14: 88026000c138 R15: 0001 [ 35.682123] FS: 7f3bbca90600() GS:8802b9d8() knlGS: [ 35.682126] CS: 0010 DS: ES: CR0: 80050033 [ 35.682130] CR2: 7f838a7b1158 CR3: 00026cbe2003 CR4: 003606e0 [ 35.682133] Call Trace: [ 35.682148] ? __mutex_unlock_slowpath+0x46/0x2b0 [ 35.682215]
Re: [Intel-gfx] [PATCH v3 3/3] drm/i195: spell out reverse on for_each macros
On Tue, 13 Nov 2018, Lucas De Marchi wrote: > Do like it's done for list.h macros, and use "reverse" suffix rather > than "rev". > > Signed-off-by: Lucas De Marchi Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_display.h| 6 +++--- > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.h > b/drivers/gpu/drm/i915/intel_display.h > index 5d50decbcbb5..43eb4ebbcc35 100644 > --- a/drivers/gpu/drm/i915/intel_display.h > +++ b/drivers/gpu/drm/i915/intel_display.h > @@ -368,7 +368,7 @@ struct intel_link_m_n { > (__dev_priv)->power_domains.power_well_count; \ >(__power_well)++) > > -#define for_each_power_well_rev(__dev_priv, __power_well) > \ > +#define for_each_power_well_reverse(__dev_priv, __power_well) > \ > for ((__power_well) = (__dev_priv)->power_domains.power_wells + > \ > (__dev_priv)->power_domains.power_well_count - 1; > \ >(__power_well) - (__dev_priv)->power_domains.power_wells >= 0; > \ > @@ -378,8 +378,8 @@ struct intel_link_m_n { > for_each_power_well(__dev_priv, __power_well) > \ > for_each_if((__power_well)->desc->domains & (__domain_mask)) > > -#define for_each_power_domain_well_rev(__dev_priv, __power_well, > __domain_mask) \ > - for_each_power_well_rev(__dev_priv, __power_well) > \ > +#define for_each_power_domain_well_reverse(__dev_priv, __power_well, > __domain_mask) \ > + for_each_power_well_reverse(__dev_priv, __power_well) > \ > for_each_if((__power_well)->desc->domains & (__domain_mask)) > > #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, > __i) \ > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index f945db6ea420..7aa5f07f1356 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -208,7 +208,7 @@ bool __intel_display_power_is_enabled(struct > drm_i915_private *dev_priv, > > is_enabled = true; > > - for_each_power_domain_well_rev(dev_priv, power_well, BIT_ULL(domain)) { > + for_each_power_domain_well_reverse(dev_priv, power_well, > BIT_ULL(domain)) { > if (power_well->desc->always_on) > continue; > > @@ -1651,7 +1651,7 @@ void intel_display_power_put(struct drm_i915_private > *dev_priv, >intel_display_power_domain_str(domain)); > power_domains->domain_use_count[domain]--; > > - for_each_power_domain_well_rev(dev_priv, power_well, BIT_ULL(domain)) > + for_each_power_domain_well_reverse(dev_priv, power_well, > BIT_ULL(domain)) > intel_power_well_put(dev_priv, power_well); > > mutex_unlock(_domains->lock); -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/icl: reverse uninit order
On Tue, 13 Nov 2018, Lucas De Marchi wrote: > Bspec 21257 says "DDIA PHY is the comp master, so it must > not be un-initialized if other combo PHYs are in use". Here > we are shutting down all phys, so it's not strictly required. > However let's be consistent on deinitializing things in the > reversed order we initialized them. > > v2: simplify protection for enum port being unsigned in future > v3: spell out reverse rather than rev > > Signed-off-by: Lucas De Marchi > Reviewed-by: Imre Deak Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_combo_phy.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c > b/drivers/gpu/drm/i915/intel_combo_phy.c > index 49f3a533860d..3d0271cebf99 100644 > --- a/drivers/gpu/drm/i915/intel_combo_phy.c > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c > @@ -9,6 +9,10 @@ > for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++) \ > for_each_if(intel_port_is_combophy(__dev_priv, __port)) > > +#define for_each_combo_port_reverse(__dev_priv, __port) \ > + for ((__port) = I915_MAX_PORTS; (__port)-- > PORT_A;) \ > + for_each_if(intel_port_is_combophy(__dev_priv, __port)) > + > enum { > PROCMON_0_85V_DOT_0, > PROCMON_0_95V_DOT_0, > @@ -232,7 +236,7 @@ void icl_combo_phys_uninit(struct drm_i915_private > *dev_priv) > { > enum port port; > > - for_each_combo_port(dev_priv, port) { > + for_each_combo_port_reverse(dev_priv, port) { > u32 val; > > if (!icl_combo_phy_verify_state(dev_priv, port)) -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/3] drm/i915/icl: replace check for combo phy
On Tue, 13 Nov 2018, Lucas De Marchi wrote: > These are the only places that assume ports A and B are the ones with > combo phy. Let's use intel_port_is_combophy() there to make sure > it checks for combo phy ports the same way everywhere. > > v2: define for_each_combo_port() helper to check the ports > > Signed-off-by: Lucas De Marchi > Reviewed-by: Imre Deak Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_combo_phy.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_combo_phy.c > b/drivers/gpu/drm/i915/intel_combo_phy.c > index f7c16f6724f0..49f3a533860d 100644 > --- a/drivers/gpu/drm/i915/intel_combo_phy.c > +++ b/drivers/gpu/drm/i915/intel_combo_phy.c > @@ -5,6 +5,10 @@ > > #include "intel_drv.h" > > +#define for_each_combo_port(__dev_priv, __port) \ > + for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++) \ > + for_each_if(intel_port_is_combophy(__dev_priv, __port)) > + > enum { > PROCMON_0_85V_DOT_0, > PROCMON_0_95V_DOT_0, > @@ -199,7 +203,7 @@ void icl_combo_phys_init(struct drm_i915_private > *dev_priv) > { > enum port port; > > - for (port = PORT_A; port <= PORT_B; port++) { > + for_each_combo_port(dev_priv, port) { > u32 val; > > if (icl_combo_phy_verify_state(dev_priv, port)) { > @@ -228,7 +232,7 @@ void icl_combo_phys_uninit(struct drm_i915_private > *dev_priv) > { > enum port port; > > - for (port = PORT_A; port <= PORT_B; port++) { > + for_each_combo_port(dev_priv, port) { > u32 val; > > if (!icl_combo_phy_verify_state(dev_priv, port)) -- Jani Nikula, Intel Open Source Graphics Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx