All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc()
@ 2019-04-12 18:30 Ville Syrjala
  2019-04-12 18:30 ` [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used Ville Syrjala
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Ville Syrjala @ 2019-04-12 18:30 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Move the platform checks out from haswell_set_pipemisc() and
rename it to bdw_set_pipemisc() to make it clear when to call it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 68 ++++++++++++++--------------
 1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c780ca6cd667..490bd49ff42a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -120,7 +120,7 @@ static void intel_cpu_transcoder_set_m_n(const struct intel_crtc_state *crtc_sta
 static void i9xx_set_pipeconf(const struct intel_crtc_state *crtc_state);
 static void ironlake_set_pipeconf(const struct intel_crtc_state *crtc_state);
 static void haswell_set_pipeconf(const struct intel_crtc_state *crtc_state);
-static void haswell_set_pipemisc(const struct intel_crtc_state *crtc_state);
+static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state);
 static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
@@ -5987,7 +5987,8 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 	if (!transcoder_is_dsi(cpu_transcoder))
 		haswell_set_pipeconf(pipe_config);
 
-	haswell_set_pipemisc(pipe_config);
+	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+		bdw_set_pipemisc(pipe_config);
 
 	intel_crtc->active = true;
 
@@ -8833,45 +8834,42 @@ static void haswell_set_pipeconf(const struct intel_crtc_state *crtc_state)
 	POSTING_READ(PIPECONF(cpu_transcoder));
 }
 
-static void haswell_set_pipemisc(const struct intel_crtc_state *crtc_state)
+static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
 {
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
-
-	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
-		u32 val = 0;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 val = 0;
 
-		switch (crtc_state->pipe_bpp) {
-		case 18:
-			val |= PIPEMISC_DITHER_6_BPC;
-			break;
-		case 24:
-			val |= PIPEMISC_DITHER_8_BPC;
-			break;
-		case 30:
-			val |= PIPEMISC_DITHER_10_BPC;
-			break;
-		case 36:
-			val |= PIPEMISC_DITHER_12_BPC;
-			break;
-		default:
-			/* Case prevented by pipe_config_set_bpp. */
-			BUG();
-		}
+	switch (crtc_state->pipe_bpp) {
+	case 18:
+		val |= PIPEMISC_DITHER_6_BPC;
+		break;
+	case 24:
+		val |= PIPEMISC_DITHER_8_BPC;
+		break;
+	case 30:
+		val |= PIPEMISC_DITHER_10_BPC;
+		break;
+	case 36:
+		val |= PIPEMISC_DITHER_12_BPC;
+		break;
+	default:
+		MISSING_CASE(crtc_state->pipe_bpp);
+		break;
+	}
 
-		if (crtc_state->dither)
-			val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
+	if (crtc_state->dither)
+		val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
 
-		if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-		    crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
-			val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+	    crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR444)
+		val |= PIPEMISC_OUTPUT_COLORSPACE_YUV;
 
-		if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
-			val |= PIPEMISC_YUV420_ENABLE |
-				PIPEMISC_YUV420_MODE_FULL_BLEND;
+	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
+		val |= PIPEMISC_YUV420_ENABLE |
+			PIPEMISC_YUV420_MODE_FULL_BLEND;
 
-		I915_WRITE(PIPEMISC(intel_crtc->pipe), val);
-	}
+	I915_WRITE(PIPEMISC(crtc->pipe), val);
 }
 
 int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-12 18:30 [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Ville Syrjala
@ 2019-04-12 18:30 ` Ville Syrjala
  2019-04-26 13:10   ` Sharma, Shashank
  2019-04-12 18:34 ` [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjala @ 2019-04-12 18:30 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The pipe has a special HDR mode with higher precision when only
HDR planes are active. Let's use it.

Curiously this fixes the kms_color gamma/degamma tests when
using a HDR plane, which is always the case unless one hacks
the test to use an SDR plane. If one does hack the test to use
an SDR plane it does pass already.

I have no actual explanation how the output after the gamma
LUT can be different between the two modes. The way the tests
are written should mean that the output should be identical
between the solid color vs. the gradient. But clearly that
somehow doesn't hold true for the HDR planes in non-HDR pipe
mode. Anyways, as long as we stick to one type of plane the
test should produce sensible results now.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  7 +++++++
 drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8ad2f0a03f28..90d60ecd3317 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5767,6 +5767,7 @@ enum {
 #define _PIPE_MISC_B			0x71030
 #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
 #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
+#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
 #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
 #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
 #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 490bd49ff42a..d0dbdbd5db3f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
 			ironlake_pfit_disable(old_crtc_state);
 	}
 
+	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+		bdw_set_pipemisc(new_crtc_state);
+
 	if (INTEL_GEN(dev_priv) >= 11)
 		icl_set_pipe_chicken(crtc);
 }
@@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
 		val |= PIPEMISC_YUV420_ENABLE |
 			PIPEMISC_YUV420_MODE_FULL_BLEND;
 
+	if (INTEL_GEN(dev_priv) >= 11 &&
+	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
+		val |= PIPEMISC_HDR_MODE;
+
 	I915_WRITE(PIPEMISC(crtc->pipe), val);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.h b/drivers/gpu/drm/i915/intel_sprite.h
index 84be8686be16..500f6bffb139 100644
--- a/drivers/gpu/drm/i915/intel_sprite.h
+++ b/drivers/gpu/drm/i915/intel_sprite.h
@@ -43,13 +43,17 @@ static inline bool icl_is_nv12_y_plane(enum plane_id id)
 	return false;
 }
 
+static inline u8 icl_hdr_plane_mask(void)
+{
+	return BIT(PLANE_PRIMARY) |
+		BIT(PLANE_SPRITE0) | BIT(PLANE_SPRITE1);
+}
+
 static inline bool icl_is_hdr_plane(struct drm_i915_private *dev_priv,
 				    enum plane_id plane_id)
 {
-	if (INTEL_GEN(dev_priv) < 11)
-		return false;
-
-	return plane_id < PLANE_SPRITE2;
+	return INTEL_GEN(dev_priv) >= 11 &&
+		icl_hdr_plane_mask() & BIT(plane_id);
 }
 
 #endif /* __INTEL_SPRITE_H__ */
-- 
2.21.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc()
  2019-04-12 18:30 [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Ville Syrjala
  2019-04-12 18:30 ` [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used Ville Syrjala
@ 2019-04-12 18:34 ` Chris Wilson
  2019-04-12 19:41 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
  2019-04-12 23:27 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-04-12 18:34 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Quoting Ville Syrjala (2019-04-12 19:30:08)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the platform checks out from haswell_set_pipemisc() and
> rename it to bdw_set_pipemisc() to make it clear when to call it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Flatten and rename haswell_set_pipemisc()
  2019-04-12 18:30 [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Ville Syrjala
  2019-04-12 18:30 ` [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used Ville Syrjala
  2019-04-12 18:34 ` [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Chris Wilson
@ 2019-04-12 19:41 ` Patchwork
  2019-04-12 23:27 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-12 19:41 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Flatten and rename haswell_set_pipemisc()
URL   : https://patchwork.freedesktop.org/series/59419/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5924 -> Patchwork_12784
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-compute0:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@basic-bsd2:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109289] +1

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_psr@primary_mmap_gtt:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#110189] +3

  * igt@prime_vgem@basic-fence-flip:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109294]

  
#### Possible fixes ####

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189


Participating hosts (47 -> 41)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (7): fi-bdw-5557u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-whl-u fi-blb-e6850 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5924 -> Patchwork_12784

  CI_DRM_5924: 1f6d0d97a9eff3abfe9201a735e96ef2d634d6d9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4945: a52cc643cfe6733465cfc9ccb3d21cbdc4fd7506 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12784: bdebc50b6bec6816c6a0a5df253a42b7753ee622 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bdebc50b6bec drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
64aa3a41e3de drm/i915: Flatten and rename haswell_set_pipemisc()

== Logs ==

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Flatten and rename haswell_set_pipemisc()
  2019-04-12 18:30 [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-04-12 19:41 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
@ 2019-04-12 23:27 ` Patchwork
  3 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-04-12 23:27 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Flatten and rename haswell_set_pipemisc()
URL   : https://patchwork.freedesktop.org/series/59419/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5924_full -> Patchwork_12784_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parse@basic-rejected:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289]

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +4

  * igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd1:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +22

  * igt@gem_pwrite@huge-cpu-fbr:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109290]

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107807]

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +2

  * igt@kms_atomic_transition@3x-modeset-transitions-fencing:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +1

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-d:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-iclb:         PASS -> FAIL [fdo#103232] +21

  * igt@kms_fbcon_fbt@fbc:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109593]

  * igt@kms_flip@2x-flip-vs-rmfb-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +3

  * igt@kms_flip@flip-vs-suspend:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#108566]

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          NOTRUN -> FAIL [fdo#100368]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-indfb-msflip-blt:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +8

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-shrfb-pgflip-blt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +6

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +16

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-move:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +114

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_lowres@pipe-b-tiling-none:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-iclb:         PASS -> FAIL [fdo#110037] +10

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-iclb:         NOTRUN -> FAIL [fdo#110037]

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +2

  * igt@kms_psr@sprite_render:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +3

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_universal_plane@disable-primary-vs-flip-pipe-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +7

  * igt@prime_nv_api@nv_self_import_to_different_fd:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291]

  * igt@runner@aborted:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109593]

  
#### Possible fixes ####

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +5

  * igt@i915_pm_rpm@gem-execbuf-stress:
    - shard-skl:          INCOMPLETE [fdo#107803] / [fdo#107807] -> PASS

  * igt@kms_color@pipe-b-gamma:
    - shard-iclb:         FAIL [fdo#104782] -> PASS +5

  * igt@kms_cursor_edge_walk@pipe-a-128x128-top-edge:
    - shard-snb:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
    - shard-snb:          SKIP [fdo#109271] -> PASS +2

  * igt@kms_cursor_legacy@pipe-c-single-bo:
    - shard-kbl:          DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +11

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-apl:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +14

  * igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +3

  * igt@kms_setmode@basic:
    - shard-iclb:         FAIL [fdo#99912] -> PASS

  * igt@kms_vblank@invalid:
    - shard-kbl:          DMESG-FAIL [fdo#103558] / [fdo#105602] -> PASS

  
#### Warnings ####

  * igt@kms_lease@setcrtc_implicit_plane:
    - shard-kbl:          DMESG-FAIL [fdo#103558] / [fdo#105602] -> FAIL [fdo#110281]

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110037]: https://bugs.freedesktop.org/show_bug.cgi?id=110037
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5924 -> Patchwork_12784

  CI_DRM_5924: 1f6d0d97a9eff3abfe9201a735e96ef2d634d6d9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4945: a52cc643cfe6733465cfc9ccb3d21cbdc4fd7506 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12784: bdebc50b6bec6816c6a0a5df253a42b7753ee622 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-12 18:30 ` [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used Ville Syrjala
@ 2019-04-26 13:10   ` Sharma, Shashank
  2019-04-26 14:37     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Sharma, Shashank @ 2019-04-26 13:10 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx


On 4/13/2019 12:00 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The pipe has a special HDR mode with higher precision when only
> HDR planes are active. Let's use it.
>
> Curiously this fixes the kms_color gamma/degamma tests when
> using a HDR plane, which is always the case unless one hacks
> the test to use an SDR plane. If one does hack the test to use
> an SDR plane it does pass already.
>
> I have no actual explanation how the output after the gamma
> LUT can be different between the two modes. The way the tests
> are written should mean that the output should be identical
> between the solid color vs. the gradient. But clearly that
> somehow doesn't hold true for the HDR planes in non-HDR pipe
> mode. Anyways, as long as we stick to one type of plane the
> test should produce sensible results now.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h      |  1 +
>   drivers/gpu/drm/i915/intel_display.c |  7 +++++++
>   drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
>   3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8ad2f0a03f28..90d60ecd3317 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5767,6 +5767,7 @@ enum {
>   #define _PIPE_MISC_B			0x71030
>   #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
>   #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
> +#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
>   #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
>   #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
>   #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 490bd49ff42a..d0dbdbd5db3f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
>   			ironlake_pfit_disable(old_crtc_state);
>   	}
>   
> +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> +		bdw_set_pipemisc(new_crtc_state);
> +
>   	if (INTEL_GEN(dev_priv) >= 11)
>   		icl_set_pipe_chicken(crtc);
>   }
> @@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
>   		val |= PIPEMISC_YUV420_ENABLE |
>   			PIPEMISC_YUV420_MODE_FULL_BLEND;
>   
> +	if (INTEL_GEN(dev_priv) >= 11 &&
> +	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
> +		val |= PIPEMISC_HDR_MODE;
> +

Shouldn't we check if the content being played on plane is HDR before 
enabling this bit (even though I am not sure if there is any harm in 
doing that)? Or maybe check the connector->output_hdr_metadata ? Most of 
the times we would be sending SDR buffers on this plane. What happens 
exactly when we set this bit ? The bspec says:

"This field enables the HDR mode, allowing for higher precision output 
from the HDR supporting planes and bypassing the SDR planes in blending. "

- Shashank

>   	I915_WRITE(PIPEMISC(crtc->pipe), val);
>   }
>   
> diff --git a/drivers/gpu/drm/i915/intel_sprite.h b/drivers/gpu/drm/i915/intel_sprite.h
> index 84be8686be16..500f6bffb139 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.h
> +++ b/drivers/gpu/drm/i915/intel_sprite.h
> @@ -43,13 +43,17 @@ static inline bool icl_is_nv12_y_plane(enum plane_id id)
>   	return false;
>   }
>   
> +static inline u8 icl_hdr_plane_mask(void)
> +{
> +	return BIT(PLANE_PRIMARY) |
> +		BIT(PLANE_SPRITE0) | BIT(PLANE_SPRITE1);
> +}
> +
>   static inline bool icl_is_hdr_plane(struct drm_i915_private *dev_priv,
>   				    enum plane_id plane_id)
>   {
> -	if (INTEL_GEN(dev_priv) < 11)
> -		return false;
> -
> -	return plane_id < PLANE_SPRITE2;
> +	return INTEL_GEN(dev_priv) >= 11 &&
> +		icl_hdr_plane_mask() & BIT(plane_id);
>   }
>   
>   #endif /* __INTEL_SPRITE_H__ */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-26 13:10   ` Sharma, Shashank
@ 2019-04-26 14:37     ` Ville Syrjälä
  2019-04-30  4:52       ` Sharma, Shashank
  2019-04-30  9:17       ` Shankar, Uma
  0 siblings, 2 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-04-26 14:37 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: intel-gfx

On Fri, Apr 26, 2019 at 06:40:11PM +0530, Sharma, Shashank wrote:
> 
> On 4/13/2019 12:00 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The pipe has a special HDR mode with higher precision when only
> > HDR planes are active. Let's use it.
> >
> > Curiously this fixes the kms_color gamma/degamma tests when
> > using a HDR plane, which is always the case unless one hacks
> > the test to use an SDR plane. If one does hack the test to use
> > an SDR plane it does pass already.
> >
> > I have no actual explanation how the output after the gamma
> > LUT can be different between the two modes. The way the tests
> > are written should mean that the output should be identical
> > between the solid color vs. the gradient. But clearly that
> > somehow doesn't hold true for the HDR planes in non-HDR pipe
> > mode. Anyways, as long as we stick to one type of plane the
> > test should produce sensible results now.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h      |  1 +
> >   drivers/gpu/drm/i915/intel_display.c |  7 +++++++
> >   drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
> >   3 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8ad2f0a03f28..90d60ecd3317 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5767,6 +5767,7 @@ enum {
> >   #define _PIPE_MISC_B			0x71030
> >   #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
> >   #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
> > +#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
> >   #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
> >   #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
> >   #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 490bd49ff42a..d0dbdbd5db3f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> >   			ironlake_pfit_disable(old_crtc_state);
> >   	}
> >   
> > +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> > +		bdw_set_pipemisc(new_crtc_state);
> > +
> >   	if (INTEL_GEN(dev_priv) >= 11)
> >   		icl_set_pipe_chicken(crtc);
> >   }
> > @@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
> >   		val |= PIPEMISC_YUV420_ENABLE |
> >   			PIPEMISC_YUV420_MODE_FULL_BLEND;
> >   
> > +	if (INTEL_GEN(dev_priv) >= 11 &&
> > +	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
> > +		val |= PIPEMISC_HDR_MODE;
> > +
> 
> Shouldn't we check if the content being played on plane is HDR before 
> enabling this bit (even though I am not sure if there is any harm in 
> doing that)? Or maybe check the connector->output_hdr_metadata ? Most of 
> the times we would be sending SDR buffers on this plane. What happens 
> exactly when we set this bit ? The bspec says:
> 
> "This field enables the HDR mode, allowing for higher precision output 
> from the HDR supporting planes and bypassing the SDR planes in blending. "

I think the bit is just misnamed (like most things with "HDR" in their
name). It's just a "gimme moar precision" bit.


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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-26 14:37     ` Ville Syrjälä
@ 2019-04-30  4:52       ` Sharma, Shashank
  2019-04-30 10:39         ` Ville Syrjälä
  2019-04-30  9:17       ` Shankar, Uma
  1 sibling, 1 reply; 12+ messages in thread
From: Sharma, Shashank @ 2019-04-30  4:52 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


On 4/26/2019 8:07 PM, Ville Syrjälä wrote:
> On Fri, Apr 26, 2019 at 06:40:11PM +0530, Sharma, Shashank wrote:
>> On 4/13/2019 12:00 AM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> The pipe has a special HDR mode with higher precision when only
>>> HDR planes are active. Let's use it.
>>>
>>> Curiously this fixes the kms_color gamma/degamma tests when
>>> using a HDR plane, which is always the case unless one hacks
>>> the test to use an SDR plane. If one does hack the test to use
>>> an SDR plane it does pass already.
>>>
>>> I have no actual explanation how the output after the gamma
>>> LUT can be different between the two modes. The way the tests
>>> are written should mean that the output should be identical
>>> between the solid color vs. the gradient. But clearly that
>>> somehow doesn't hold true for the HDR planes in non-HDR pipe
>>> mode. Anyways, as long as we stick to one type of plane the
>>> test should produce sensible results now.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_reg.h      |  1 +
>>>    drivers/gpu/drm/i915/intel_display.c |  7 +++++++
>>>    drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
>>>    3 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index 8ad2f0a03f28..90d60ecd3317 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -5767,6 +5767,7 @@ enum {
>>>    #define _PIPE_MISC_B			0x71030
>>>    #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
>>>    #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
>>> +#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
>>>    #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
>>>    #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
>>>    #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 490bd49ff42a..d0dbdbd5db3f 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
>>>    			ironlake_pfit_disable(old_crtc_state);
>>>    	}
>>>    
>>> +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
>>> +		bdw_set_pipemisc(new_crtc_state);
>>> +
>>>    	if (INTEL_GEN(dev_priv) >= 11)
>>>    		icl_set_pipe_chicken(crtc);
>>>    }
>>> @@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
>>>    		val |= PIPEMISC_YUV420_ENABLE |
>>>    			PIPEMISC_YUV420_MODE_FULL_BLEND;
>>>    
>>> +	if (INTEL_GEN(dev_priv) >= 11 &&
>>> +	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
>>> +		val |= PIPEMISC_HDR_MODE;
>>> +
>> Shouldn't we check if the content being played on plane is HDR before
>> enabling this bit (even though I am not sure if there is any harm in
>> doing that)? Or maybe check the connector->output_hdr_metadata ? Most of
>> the times we would be sending SDR buffers on this plane. What happens
>> exactly when we set this bit ? The bspec says:
>>
>> "This field enables the HDR mode, allowing for higher precision output
>> from the HDR supporting planes and bypassing the SDR planes in blending. "
> I think the bit is just misnamed (like most things with "HDR" in their
> name). It's just a "gimme moar precision" bit.

Lets make this a bit more clear, may be rename the bit to 
PIPEMISC_HDR_PRECISION_MODE instead?

With that change, this patch is

Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-26 14:37     ` Ville Syrjälä
  2019-04-30  4:52       ` Sharma, Shashank
@ 2019-04-30  9:17       ` Shankar, Uma
  1 sibling, 0 replies; 12+ messages in thread
From: Shankar, Uma @ 2019-04-30  9:17 UTC (permalink / raw)
  To: Ville Syrjälä, Sharma, Shashank; +Cc: intel-gfx



>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Ville
>Syrjälä
>Sent: Friday, April 26, 2019 8:07 PM
>To: Sharma, Shashank <shashank.sharma@intel.com>
>Cc: intel-gfx@lists.freedesktop.org
>Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only
>HDR planes are used
>
>On Fri, Apr 26, 2019 at 06:40:11PM +0530, Sharma, Shashank wrote:
>>
>> On 4/13/2019 12:00 AM, Ville Syrjala wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > The pipe has a special HDR mode with higher precision when only HDR
>> > planes are active. Let's use it.
>> >
>> > Curiously this fixes the kms_color gamma/degamma tests when using a
>> > HDR plane, which is always the case unless one hacks the test to use
>> > an SDR plane. If one does hack the test to use an SDR plane it does
>> > pass already.
>> >
>> > I have no actual explanation how the output after the gamma LUT can
>> > be different between the two modes. The way the tests are written
>> > should mean that the output should be identical between the solid
>> > color vs. the gradient. But clearly that somehow doesn't hold true
>> > for the HDR planes in non-HDR pipe mode. Anyways, as long as we
>> > stick to one type of plane the test should produce sensible results
>> > now.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >   drivers/gpu/drm/i915/i915_reg.h      |  1 +
>> >   drivers/gpu/drm/i915/intel_display.c |  7 +++++++
>> >   drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
>> >   3 files changed, 16 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > b/drivers/gpu/drm/i915/i915_reg.h index 8ad2f0a03f28..90d60ecd3317
>> > 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -5767,6 +5767,7 @@ enum {
>> >   #define _PIPE_MISC_B			0x71030
>> >   #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
>> >   #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
>> > +#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
>> >   #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
>> >   #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
>> >   #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index 490bd49ff42a..d0dbdbd5db3f 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct
>intel_crtc_state *old_crtc_sta
>> >   			ironlake_pfit_disable(old_crtc_state);
>> >   	}
>> >
>> > +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
>> > +		bdw_set_pipemisc(new_crtc_state);
>> > +
>> >   	if (INTEL_GEN(dev_priv) >= 11)
>> >   		icl_set_pipe_chicken(crtc);
>> >   }
>> > @@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct
>intel_crtc_state *crtc_state)
>> >   		val |= PIPEMISC_YUV420_ENABLE |
>> >   			PIPEMISC_YUV420_MODE_FULL_BLEND;
>> >
>> > +	if (INTEL_GEN(dev_priv) >= 11 &&
>> > +	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
>> > +		val |= PIPEMISC_HDR_MODE;
>> > +
>>
>> Shouldn't we check if the content being played on plane is HDR before
>> enabling this bit (even though I am not sure if there is any harm in
>> doing that)? Or maybe check the connector->output_hdr_metadata ? Most
>> of the times we would be sending SDR buffers on this plane. What
>> happens exactly when we set this bit ? The bspec says:
>>
>> "This field enables the HDR mode, allowing for higher precision output
>> from the HDR supporting planes and bypassing the SDR planes in blending. "
>
>I think the bit is just misnamed (like most things with "HDR" in their name). It's just a
>"gimme moar precision" bit.

Yeah AFAIU this bit just enables pipe to work at higher precision mode which should be ok
if we actually require lower precision (SDR cases) and shouldn't cause any problem. And will
be a must if actual HDR data is processed on the planes which will require higher precision. 

However enabling this always for HDR planes irrespective of content is actually fixing the crc
errors.

This patch is 
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
And
Tested-by: Uma Shankar <uma.shankar@intel.com>

>
>--
>Ville Syrjälä
>Intel
>_______________________________________________
>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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-30  4:52       ` Sharma, Shashank
@ 2019-04-30 10:39         ` Ville Syrjälä
  2019-04-30 13:43           ` Sharma, Shashank
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2019-04-30 10:39 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: intel-gfx

On Tue, Apr 30, 2019 at 10:22:40AM +0530, Sharma, Shashank wrote:
> 
> On 4/26/2019 8:07 PM, Ville Syrjälä wrote:
> > On Fri, Apr 26, 2019 at 06:40:11PM +0530, Sharma, Shashank wrote:
> >> On 4/13/2019 12:00 AM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> The pipe has a special HDR mode with higher precision when only
> >>> HDR planes are active. Let's use it.
> >>>
> >>> Curiously this fixes the kms_color gamma/degamma tests when
> >>> using a HDR plane, which is always the case unless one hacks
> >>> the test to use an SDR plane. If one does hack the test to use
> >>> an SDR plane it does pass already.
> >>>
> >>> I have no actual explanation how the output after the gamma
> >>> LUT can be different between the two modes. The way the tests
> >>> are written should mean that the output should be identical
> >>> between the solid color vs. the gradient. But clearly that
> >>> somehow doesn't hold true for the HDR planes in non-HDR pipe
> >>> mode. Anyways, as long as we stick to one type of plane the
> >>> test should produce sensible results now.
> >>>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/i915_reg.h      |  1 +
> >>>    drivers/gpu/drm/i915/intel_display.c |  7 +++++++
> >>>    drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
> >>>    3 files changed, 16 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>> index 8ad2f0a03f28..90d60ecd3317 100644
> >>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >>> @@ -5767,6 +5767,7 @@ enum {
> >>>    #define _PIPE_MISC_B			0x71030
> >>>    #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
> >>>    #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
> >>> +#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
> >>>    #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
> >>>    #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
> >>>    #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> index 490bd49ff42a..d0dbdbd5db3f 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> >>>    			ironlake_pfit_disable(old_crtc_state);
> >>>    	}
> >>>    
> >>> +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> >>> +		bdw_set_pipemisc(new_crtc_state);
> >>> +
> >>>    	if (INTEL_GEN(dev_priv) >= 11)
> >>>    		icl_set_pipe_chicken(crtc);
> >>>    }
> >>> @@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
> >>>    		val |= PIPEMISC_YUV420_ENABLE |
> >>>    			PIPEMISC_YUV420_MODE_FULL_BLEND;
> >>>    
> >>> +	if (INTEL_GEN(dev_priv) >= 11 &&
> >>> +	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
> >>> +		val |= PIPEMISC_HDR_MODE;
> >>> +
> >> Shouldn't we check if the content being played on plane is HDR before
> >> enabling this bit (even though I am not sure if there is any harm in
> >> doing that)? Or maybe check the connector->output_hdr_metadata ? Most of
> >> the times we would be sending SDR buffers on this plane. What happens
> >> exactly when we set this bit ? The bspec says:
> >>
> >> "This field enables the HDR mode, allowing for higher precision output
> >> from the HDR supporting planes and bypassing the SDR planes in blending. "
> > I think the bit is just misnamed (like most things with "HDR" in their
> > name). It's just a "gimme moar precision" bit.
> 
> Lets make this a bit more clear, may be rename the bit to 
> PIPEMISC_HDR_PRECISION_MODE instead?

Then it won't match the spec.

> 
> With that change, this patch is
> 
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> 
> >

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-30 10:39         ` Ville Syrjälä
@ 2019-04-30 13:43           ` Sharma, Shashank
  2019-04-30 19:19             ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Sharma, Shashank @ 2019-04-30 13:43 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


On 4/30/2019 4:09 PM, Ville Syrjälä wrote:
> On Tue, Apr 30, 2019 at 10:22:40AM +0530, Sharma, Shashank wrote:
>> On 4/26/2019 8:07 PM, Ville Syrjälä wrote:
>>> On Fri, Apr 26, 2019 at 06:40:11PM +0530, Sharma, Shashank wrote:
>>>> On 4/13/2019 12:00 AM, Ville Syrjala wrote:
>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>
>>>>> The pipe has a special HDR mode with higher precision when only
>>>>> HDR planes are active. Let's use it.
>>>>>
>>>>> Curiously this fixes the kms_color gamma/degamma tests when
>>>>> using a HDR plane, which is always the case unless one hacks
>>>>> the test to use an SDR plane. If one does hack the test to use
>>>>> an SDR plane it does pass already.
>>>>>
>>>>> I have no actual explanation how the output after the gamma
>>>>> LUT can be different between the two modes. The way the tests
>>>>> are written should mean that the output should be identical
>>>>> between the solid color vs. the gradient. But clearly that
>>>>> somehow doesn't hold true for the HDR planes in non-HDR pipe
>>>>> mode. Anyways, as long as we stick to one type of plane the
>>>>> test should produce sensible results now.
>>>>>
>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>> ---
>>>>>     drivers/gpu/drm/i915/i915_reg.h      |  1 +
>>>>>     drivers/gpu/drm/i915/intel_display.c |  7 +++++++
>>>>>     drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
>>>>>     3 files changed, 16 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>>>> index 8ad2f0a03f28..90d60ecd3317 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>>>> @@ -5767,6 +5767,7 @@ enum {
>>>>>     #define _PIPE_MISC_B			0x71030
>>>>>     #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
>>>>>     #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
>>>>> +#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
>>>>>     #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
>>>>>     #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
>>>>>     #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>>> index 490bd49ff42a..d0dbdbd5db3f 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>>> @@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
>>>>>     			ironlake_pfit_disable(old_crtc_state);
>>>>>     	}
>>>>>     
>>>>> +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
>>>>> +		bdw_set_pipemisc(new_crtc_state);
>>>>> +
>>>>>     	if (INTEL_GEN(dev_priv) >= 11)
>>>>>     		icl_set_pipe_chicken(crtc);
>>>>>     }
>>>>> @@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
>>>>>     		val |= PIPEMISC_YUV420_ENABLE |
>>>>>     			PIPEMISC_YUV420_MODE_FULL_BLEND;
>>>>>     
>>>>> +	if (INTEL_GEN(dev_priv) >= 11 &&
>>>>> +	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
>>>>> +		val |= PIPEMISC_HDR_MODE;
>>>>> +
>>>> Shouldn't we check if the content being played on plane is HDR before
>>>> enabling this bit (even though I am not sure if there is any harm in
>>>> doing that)? Or maybe check the connector->output_hdr_metadata ? Most of
>>>> the times we would be sending SDR buffers on this plane. What happens
>>>> exactly when we set this bit ? The bspec says:
>>>>
>>>> "This field enables the HDR mode, allowing for higher precision output
>>>> from the HDR supporting planes and bypassing the SDR planes in blending. "
>>> I think the bit is just misnamed (like most things with "HDR" in their
>>> name). It's just a "gimme moar precision" bit.
>> Lets make this a bit more clear, may be rename the bit to
>> PIPEMISC_HDR_PRECISION_MODE instead?
> Then it won't match the spec.

Well we are keeping HDR_MODE aren't we ? may be 
PIPEMISC_HDR_MODE_PRECISION if that makes us closer to spec ?

- Shashank

>> With that change, this patch is
>>
>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used
  2019-04-30 13:43           ` Sharma, Shashank
@ 2019-04-30 19:19             ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2019-04-30 19:19 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: intel-gfx

On Tue, Apr 30, 2019 at 07:13:36PM +0530, Sharma, Shashank wrote:
> 
> On 4/30/2019 4:09 PM, Ville Syrjälä wrote:
> > On Tue, Apr 30, 2019 at 10:22:40AM +0530, Sharma, Shashank wrote:
> >> On 4/26/2019 8:07 PM, Ville Syrjälä wrote:
> >>> On Fri, Apr 26, 2019 at 06:40:11PM +0530, Sharma, Shashank wrote:
> >>>> On 4/13/2019 12:00 AM, Ville Syrjala wrote:
> >>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>
> >>>>> The pipe has a special HDR mode with higher precision when only
> >>>>> HDR planes are active. Let's use it.
> >>>>>
> >>>>> Curiously this fixes the kms_color gamma/degamma tests when
> >>>>> using a HDR plane, which is always the case unless one hacks
> >>>>> the test to use an SDR plane. If one does hack the test to use
> >>>>> an SDR plane it does pass already.
> >>>>>
> >>>>> I have no actual explanation how the output after the gamma
> >>>>> LUT can be different between the two modes. The way the tests
> >>>>> are written should mean that the output should be identical
> >>>>> between the solid color vs. the gradient. But clearly that
> >>>>> somehow doesn't hold true for the HDR planes in non-HDR pipe
> >>>>> mode. Anyways, as long as we stick to one type of plane the
> >>>>> test should produce sensible results now.
> >>>>>
> >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>> ---
> >>>>>     drivers/gpu/drm/i915/i915_reg.h      |  1 +
> >>>>>     drivers/gpu/drm/i915/intel_display.c |  7 +++++++
> >>>>>     drivers/gpu/drm/i915/intel_sprite.h  | 12 ++++++++----
> >>>>>     3 files changed, 16 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> >>>>> index 8ad2f0a03f28..90d60ecd3317 100644
> >>>>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >>>>> @@ -5767,6 +5767,7 @@ enum {
> >>>>>     #define _PIPE_MISC_B			0x71030
> >>>>>     #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
> >>>>>     #define   PIPEMISC_YUV420_MODE_FULL_BLEND (1 << 26)
> >>>>> +#define   PIPEMISC_HDR_MODE		(1 << 23) /* icl+ */
> >>>>>     #define   PIPEMISC_OUTPUT_COLORSPACE_YUV  (1 << 11)
> >>>>>     #define   PIPEMISC_DITHER_BPC_MASK	(7 << 5)
> >>>>>     #define   PIPEMISC_DITHER_8_BPC		(0 << 5)
> >>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>>> index 490bd49ff42a..d0dbdbd5db3f 100644
> >>>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>>> @@ -4055,6 +4055,9 @@ static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_sta
> >>>>>     			ironlake_pfit_disable(old_crtc_state);
> >>>>>     	}
> >>>>>     
> >>>>> +	if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> >>>>> +		bdw_set_pipemisc(new_crtc_state);
> >>>>> +
> >>>>>     	if (INTEL_GEN(dev_priv) >= 11)
> >>>>>     		icl_set_pipe_chicken(crtc);
> >>>>>     }
> >>>>> @@ -8869,6 +8872,10 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
> >>>>>     		val |= PIPEMISC_YUV420_ENABLE |
> >>>>>     			PIPEMISC_YUV420_MODE_FULL_BLEND;
> >>>>>     
> >>>>> +	if (INTEL_GEN(dev_priv) >= 11 &&
> >>>>> +	    (crtc_state->active_planes & ~icl_hdr_plane_mask()) == 0)
> >>>>> +		val |= PIPEMISC_HDR_MODE;
> >>>>> +
> >>>> Shouldn't we check if the content being played on plane is HDR before
> >>>> enabling this bit (even though I am not sure if there is any harm in
> >>>> doing that)? Or maybe check the connector->output_hdr_metadata ? Most of
> >>>> the times we would be sending SDR buffers on this plane. What happens
> >>>> exactly when we set this bit ? The bspec says:
> >>>>
> >>>> "This field enables the HDR mode, allowing for higher precision output
> >>>> from the HDR supporting planes and bypassing the SDR planes in blending. "
> >>> I think the bit is just misnamed (like most things with "HDR" in their
> >>> name). It's just a "gimme moar precision" bit.
> >> Lets make this a bit more clear, may be rename the bit to
> >> PIPEMISC_HDR_PRECISION_MODE instead?
> > Then it won't match the spec.
> 
> Well we are keeping HDR_MODE aren't we ? may be 
> PIPEMISC_HDR_MODE_PRECISION if that makes us closer to spec ?

Not super convinved that really helps the reader in any significant way,
but I changed it anyway.

Series pushed to dinq. Thanks for the reviews.

> 
> - Shashank
> 
> >> With that change, this patch is
> >>
> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> >>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-30 19:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-12 18:30 [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Ville Syrjala
2019-04-12 18:30 ` [PATCH 2/2] drm/i915: Enable pipe HDR mode on ICL if only HDR planes are used Ville Syrjala
2019-04-26 13:10   ` Sharma, Shashank
2019-04-26 14:37     ` Ville Syrjälä
2019-04-30  4:52       ` Sharma, Shashank
2019-04-30 10:39         ` Ville Syrjälä
2019-04-30 13:43           ` Sharma, Shashank
2019-04-30 19:19             ` Ville Syrjälä
2019-04-30  9:17       ` Shankar, Uma
2019-04-12 18:34 ` [PATCH 1/2] drm/i915: Flatten and rename haswell_set_pipemisc() Chris Wilson
2019-04-12 19:41 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
2019-04-12 23:27 ` ✓ Fi.CI.IGT: " Patchwork

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