intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: set DP Main Stream Attribute for color range on DDI platforms
@ 2018-08-14  6:00 Jani Nikula
  2018-08-14  6:25 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Jani Nikula @ 2018-08-14  6:00 UTC (permalink / raw)
  To: intel-gfx
  Cc: jani.nikula, Nicholas Stommel, Paulo Zanoni, Rodrigo Vivi,
	Ville Syrjälä, # v3 . 9+

Since Haswell we have no color range indication either in the pipe or
port registers for DP. Instead, there's a separate register for setting
the DP Main Stream Attributes (MSA) directly. The MSA register
definition makes no references to colorimetry, just a vague reference to
the DP spec. The connection to the color range was lost.

Apparently we've failed to set the proper MSA bit for limited, or CEA,
range ever since the first DDI platforms. We've started setting other
MSA parameters since commit dae847991a43 ("drm/i915: add
intel_ddi_set_pipe_settings").

Without the crucial bit of information, the DP sink has no way of
knowing the source is actually transmitting limited range RGB, leading
to "washed out" colors. With the colorimetry information, compliant
sinks should be able to handle the limited range properly. Native
(i.e. non-LSPCON) HDMI was not affected because we do pass the color
range via AVI infoframes.

Though not the root cause, the problem was made worse for DDI platforms
with commit 55bc60db5988 ("drm/i915: Add "Automatic" mode for the
"Broadcast RGB" property"), which selects limited range RGB
automatically based on the mode, as per the DP, HDMI and CEA specs.

After all these years, the fix boils down to flipping one bit.

[Per testing reports, this fixes DP sinks, but not the LSPCON. My
 educated guess is that the LSPCON fails to turn the CEA range MSA into
 AVI infoframes for HDMI.]

Reported-by: Michał Kopeć <mkopec12@gmail.com>
Reported-by: N. W. <nw9165-3201@yahoo.com>
Reported-by: Nicholas Stommel <nicholas.stommel@gmail.com>
Reported-by: Tom Yan <tom.ty89@gmail.com>
Tested-by: Nicholas Stommel <nicholas.stommel@gmail.com>
References: https://bugs.freedesktop.org/show_bug.cgi?id=100023
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107476
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94921
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v3.9+
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 1 +
 drivers/gpu/drm/i915/intel_ddi.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 17575cfc22b5..0c9f03dda569 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9246,6 +9246,7 @@ enum skl_power_gate {
 #define  TRANS_MSA_10_BPC		(2 << 5)
 #define  TRANS_MSA_12_BPC		(3 << 5)
 #define  TRANS_MSA_16_BPC		(4 << 5)
+#define  TRANS_MSA_CEA_RANGE		(1 << 3)
 
 /* LCPLL Control */
 #define LCPLL_CTL			_MMIO(0x130040)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0adc043529f2..6f7be066c8f2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1685,6 +1685,10 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
 	WARN_ON(transcoder_is_dsi(cpu_transcoder));
 
 	temp = TRANS_MSA_SYNC_CLK;
+
+	if (crtc_state->limited_color_range)
+		temp |= TRANS_MSA_CEA_RANGE;
+
 	switch (crtc_state->pipe_bpp) {
 	case 18:
 		temp |= TRANS_MSA_6_BPC;
-- 
2.11.0

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: set DP Main Stream Attribute for color range on DDI platforms
  2018-08-14  6:00 [PATCH] drm/i915: set DP Main Stream Attribute for color range on DDI platforms Jani Nikula
@ 2018-08-14  6:25 ` Patchwork
  2018-08-14  6:43 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2018-08-14  6:25 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: set DP Main Stream Attribute for color range on DDI platforms
URL   : https://patchwork.freedesktop.org/series/48145/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1c8e245b1a71 drm/i915: set DP Main Stream Attribute for color range on DDI platforms
-:29: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 55bc60db5988 ("drm/i915: Add "Automatic" mode for the "Broadcast RGB" property")'
#29: 
with commit 55bc60db5988 ("drm/i915: Add "Automatic" mode for the

total: 1 errors, 0 warnings, 0 checks, 17 lines checked

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

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

* ✓ Fi.CI.BAT: success for drm/i915: set DP Main Stream Attribute for color range on DDI platforms
  2018-08-14  6:00 [PATCH] drm/i915: set DP Main Stream Attribute for color range on DDI platforms Jani Nikula
  2018-08-14  6:25 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-08-14  6:43 ` Patchwork
  2018-08-14  7:32 ` ✓ Fi.CI.IGT: " Patchwork
  2018-08-14 13:21 ` [Intel-gfx] [PATCH] " Rodrigo Vivi
  3 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2018-08-14  6:43 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: set DP Main Stream Attribute for color range on DDI platforms
URL   : https://patchwork.freedesktop.org/series/48145/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4664 -> Patchwork_9933 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362, fdo#103191)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         DMESG-FAIL (fdo#107164) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         DMESG-FAIL (fdo#106685, fdo#107174) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#107139, fdo#105128) -> PASS

    igt@kms_chamelium@dp-crc-fast:
      fi-kbl-7500u:       DMESG-FAIL (fdo#103841) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {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#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106685 https://bugs.freedesktop.org/show_bug.cgi?id=106685
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (54 -> 46) ==

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u 


== Build changes ==

    * Linux: CI_DRM_4664 -> Patchwork_9933

  CI_DRM_4664: 19e458884fe1d8d10e453529933199250cc8821f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9933: 1c8e245b1a71a92c208232ddb08cd4aac7c8847f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1c8e245b1a71 drm/i915: set DP Main Stream Attribute for color range on DDI platforms

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915: set DP Main Stream Attribute for color range on DDI platforms
  2018-08-14  6:00 [PATCH] drm/i915: set DP Main Stream Attribute for color range on DDI platforms Jani Nikula
  2018-08-14  6:25 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2018-08-14  6:43 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-08-14  7:32 ` Patchwork
  2018-08-14 13:21 ` [Intel-gfx] [PATCH] " Rodrigo Vivi
  3 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2018-08-14  7:32 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: set DP Main Stream Attribute for color range on DDI platforms
URL   : https://patchwork.freedesktop.org/series/48145/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4664_full -> Patchwork_9933_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    {igt@gem_userptr_blits@readonly-unsync}:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-shrfb-plflip-blt:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

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

    igt@perf_pmu@init-wait-rcs0:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359, fdo#106886) -> PASS

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          FAIL (fdo#103355) -> PASS

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

    
    ==== Warnings ====

    igt@drv_suspend@shrink:
      shard-snb:          FAIL (fdo#106886) -> INCOMPLETE (fdo#105411, fdo#106886)

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

  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  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 (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4664 -> Patchwork_9933

  CI_DRM_4664: 19e458884fe1d8d10e453529933199250cc8821f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4593: c88e219c6e890d89b7836c5e248ffedf334d55a2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9933: 1c8e245b1a71a92c208232ddb08cd4aac7c8847f @ 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_9933/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH] drm/i915: set DP Main Stream Attribute for color range on DDI platforms
  2018-08-14  6:00 [PATCH] drm/i915: set DP Main Stream Attribute for color range on DDI platforms Jani Nikula
                   ` (2 preceding siblings ...)
  2018-08-14  7:32 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-08-14 13:21 ` Rodrigo Vivi
  3 siblings, 0 replies; 5+ messages in thread
From: Rodrigo Vivi @ 2018-08-14 13:21 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, Paulo Zanoni, Nicholas Stommel, # v3 . 9+

On Tue, Aug 14, 2018 at 09:00:01AM +0300, Jani Nikula wrote:
> Since Haswell we have no color range indication either in the pipe or
> port registers for DP. Instead, there's a separate register for setting
> the DP Main Stream Attributes (MSA) directly. The MSA register
> definition makes no references to colorimetry, just a vague reference to
> the DP spec. The connection to the color range was lost.
> 
> Apparently we've failed to set the proper MSA bit for limited, or CEA,
> range ever since the first DDI platforms. We've started setting other
> MSA parameters since commit dae847991a43 ("drm/i915: add
> intel_ddi_set_pipe_settings").
> 
> Without the crucial bit of information, the DP sink has no way of
> knowing the source is actually transmitting limited range RGB, leading
> to "washed out" colors. With the colorimetry information, compliant
> sinks should be able to handle the limited range properly. Native
> (i.e. non-LSPCON) HDMI was not affected because we do pass the color
> range via AVI infoframes.
> 
> Though not the root cause, the problem was made worse for DDI platforms
> with commit 55bc60db5988 ("drm/i915: Add "Automatic" mode for the
> "Broadcast RGB" property"), which selects limited range RGB
> automatically based on the mode, as per the DP, HDMI and CEA specs.
> 
> After all these years, the fix boils down to flipping one bit.
> 
> [Per testing reports, this fixes DP sinks, but not the LSPCON. My
>  educated guess is that the LSPCON fails to turn the CEA range MSA into
>  AVI infoframes for HDMI.]
> 
> Reported-by: Michał Kopeć <mkopec12@gmail.com>
> Reported-by: N. W. <nw9165-3201@yahoo.com>
> Reported-by: Nicholas Stommel <nicholas.stommel@gmail.com>
> Reported-by: Tom Yan <tom.ty89@gmail.com>
> Tested-by: Nicholas Stommel <nicholas.stommel@gmail.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=100023
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107476
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94921
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v3.9+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

wow! and that was hard to review as well...

I finally found the bit defined and set for this case on
"Table 2-96: MISC1 and MISC0 Fields for Pixel Encoding/Colorimetry Format Indication"

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 1 +
>  drivers/gpu/drm/i915/intel_ddi.c | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 17575cfc22b5..0c9f03dda569 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9246,6 +9246,7 @@ enum skl_power_gate {
>  #define  TRANS_MSA_10_BPC		(2 << 5)
>  #define  TRANS_MSA_12_BPC		(3 << 5)
>  #define  TRANS_MSA_16_BPC		(4 << 5)
> +#define  TRANS_MSA_CEA_RANGE		(1 << 3)
>  
>  /* LCPLL Control */
>  #define LCPLL_CTL			_MMIO(0x130040)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0adc043529f2..6f7be066c8f2 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1685,6 +1685,10 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
>  	WARN_ON(transcoder_is_dsi(cpu_transcoder));
>  
>  	temp = TRANS_MSA_SYNC_CLK;
> +
> +	if (crtc_state->limited_color_range)
> +		temp |= TRANS_MSA_CEA_RANGE;
> +
>  	switch (crtc_state->pipe_bpp) {
>  	case 18:
>  		temp |= TRANS_MSA_6_BPC;
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-08-14 13:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-14  6:00 [PATCH] drm/i915: set DP Main Stream Attribute for color range on DDI platforms Jani Nikula
2018-08-14  6:25 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-08-14  6:43 ` ✓ Fi.CI.BAT: success " Patchwork
2018-08-14  7:32 ` ✓ Fi.CI.IGT: " Patchwork
2018-08-14 13:21 ` [Intel-gfx] [PATCH] " Rodrigo Vivi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).