All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output
@ 2019-02-15 23:35 Matt Roper
  2019-02-16  0:44 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Matt Roper @ 2019-02-15 23:35 UTC (permalink / raw)
  To: intel-gfx

We recently added support for gen11's new "output csc" which can be used
independently of the regular pipe CSC.  The idea is that this new output
csc allows us to use a userspace-provided color transformation matrix at
the same time we drive a YCBCR display mode requiring RGB->YUV
conversion.  However when landing that support we only updated the color
management code and overlooked that there was an additional atomic check
in the modeset path (in intel_crtc_compute_config()) that still assumes
we only have a single CSC unit to work with.  Let's update that check to
only apply to pre-gen11 platforms and move it into the color management
code to ensure it gets called on all commits, not just modesets.

Also, if we're *only* using the output CSC and not the pipe CSC, there's
no need to set crtc_state->csc_enable, so let's also not consider the
output format when setting csc_enable on gen11+ platforms.

Fixes: a91de580541c ("drm/i915/icl: Enable pipe output csc")
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
It might also be worth moving the full->limited range RGB conversion
from the pipe CSC to the output CSC on gen11 at some point.

 drivers/gpu/drm/i915/intel_color.c   | 16 +++++++++++++---
 drivers/gpu/drm/i915/intel_display.c | 12 ------------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index da7a07d5ccea..b47e6d1aaaec 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -780,9 +780,10 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
 	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
 		limited_color_range = crtc_state->limited_color_range;
 
-	crtc_state->csc_enable =
-		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
-		crtc_state->base.ctm || limited_color_range;
+	crtc_state->csc_enable = crtc_state->base.ctm || limited_color_range;
+	if (INTEL_GEN(dev_priv) < 11)
+		crtc_state->csc_enable |=
+			crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB;
 
 	ret = intel_color_add_affected_planes(crtc_state);
 	if (ret)
@@ -822,6 +823,15 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
 			crtc_state->csc_mode |= ICL_OUTPUT_CSC_ENABLE;
 
 		crtc_state->csc_mode |= ICL_CSC_ENABLE;
+	} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB &&
+		   crtc_state->base.ctm) {
+		/*
+		 * There is only one pipe CSC unit per pipe on pre-gen11, and
+		 * we need that for output conversion from RGB->YCBCR. So if
+		 * CTM is already applied we can't support YCBCR output.
+		 */
+		DRM_DEBUG_KMS("YCBCR420 and CTM together are not possible\n");
+		return -EINVAL;
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index afa21daaae51..81bfdbd99092 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6855,18 +6855,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
-	if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
-	     pipe_config->base.ctm) {
-		/*
-		 * There is only one pipe CSC unit per pipe, and we need that
-		 * for output conversion from RGB->YCBCR. So if CTM is already
-		 * applied we can't support YCBCR420 output.
-		 */
-		DRM_DEBUG_KMS("YCBCR420 and CTM together are not possible\n");
-		return -EINVAL;
-	}
-
 	/*
 	 * Pipe horizontal size must be even in:
 	 * - DVO ganged mode
-- 
2.14.5

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

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

* ✓ Fi.CI.BAT: success for drm/i915/icl: Fix checks for userspace ctm + ycbcr output
  2019-02-15 23:35 [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output Matt Roper
@ 2019-02-16  0:44 ` Patchwork
  2019-02-16 11:34 ` ✗ Fi.CI.IGT: failure " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2019-02-16  0:44 UTC (permalink / raw)
  To: intel-gfx

== Series Details ==

Series: drm/i915/icl: Fix checks for userspace ctm + ycbcr output
URL   : https://patchwork.freedesktop.org/series/56770/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5612 -> Patchwork_12233
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  * igt@pm_rpm@basic-rte:
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108800]

  
#### Possible fixes ####

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> 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#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (45 -> 41)
------------------------------

  Additional (1): fi-glk-j4005 
  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4770 fi-icl-y 


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

    * Linux: CI_DRM_5612 -> Patchwork_12233

  CI_DRM_5612: 6067ec567b3eda30cecc8e03b65c4783f8708a8c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4832: 324ab48e67065f0cf67525b3ab9c44fd3dcaef0a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12233: b8e1a8bf7f52e1d4dbbaded78db7acf193646769 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b8e1a8bf7f52 drm/i915/icl: Fix checks for userspace ctm + ycbcr output

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12233/
_______________________________________________
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: failure for drm/i915/icl: Fix checks for userspace ctm + ycbcr output
  2019-02-15 23:35 [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output Matt Roper
  2019-02-16  0:44 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-02-16 11:34 ` Patchwork
  2019-02-18 14:28 ` [PATCH] " Shankar, Uma
  2019-02-18 20:28 ` Ville Syrjälä
  3 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2019-02-16 11:34 UTC (permalink / raw)
  To: intel-gfx

== Series Details ==

Series: drm/i915/icl: Fix checks for userspace ctm + ycbcr output
URL   : https://patchwork.freedesktop.org/series/56770/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5612_full -> Patchwork_12233_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12233_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12233_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_12233_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_color@pipe-c-legacy-gamma-reset:
    - shard-kbl:          PASS -> FAIL
    - shard-glk:          PASS -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_color@pipe-b-degamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_color@pipe-c-ctm-0-25:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#109624]

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-apl:          PASS -> FAIL [fdo#109350]

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-hsw:          PASS -> FAIL [fdo#102887]

  * igt@kms_flip@2x-plain-flip-fb-recreate:
    - shard-glk:          PASS -> FAIL [fdo#100368]

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

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-onoff:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@pm_rpm@gem-idle:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#107713] / [fdo#108840]

  
#### Possible fixes ####

  * igt@kms_color@pipe-b-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-pwrite:
    - shard-iclb:         INCOMPLETE [fdo#106978] -> PASS

  * igt@kms_lease@lease_invalid_connector:
    - shard-snb:          {SKIP} [fdo#109271] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

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

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

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         FAIL [fdo#100047] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-modeset-rpm:
    - shard-apl:          FAIL [fdo#104894] -> PASS

  * igt@pm_rpm@cursor-dpms:
    - shard-iclb:         DMESG-WARN [fdo#107724] -> PASS +3

  
#### Warnings ####

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

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

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [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#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109281]: https://bugs.freedesktop.org/show_bug.cgi?id=109281
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109624]: https://bugs.freedesktop.org/show_bug.cgi?id=109624
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 6)
------------------------------

  Missing    (1): shard-skl 


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

    * Linux: CI_DRM_5612 -> Patchwork_12233

  CI_DRM_5612: 6067ec567b3eda30cecc8e03b65c4783f8708a8c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4832: 324ab48e67065f0cf67525b3ab9c44fd3dcaef0a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12233: b8e1a8bf7f52e1d4dbbaded78db7acf193646769 @ 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_12233/
_______________________________________________
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: [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output
  2019-02-15 23:35 [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output Matt Roper
  2019-02-16  0:44 ` ✓ Fi.CI.BAT: success for " Patchwork
  2019-02-16 11:34 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-02-18 14:28 ` Shankar, Uma
  2019-02-18 20:28 ` Ville Syrjälä
  3 siblings, 0 replies; 5+ messages in thread
From: Shankar, Uma @ 2019-02-18 14:28 UTC (permalink / raw)
  To: Roper, Matthew D, intel-gfx



>-----Original Message-----
>From: Roper, Matthew D
>Sent: Saturday, February 16, 2019 5:06 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Roper, Matthew D <matthew.d.roper@intel.com>; Shankar, Uma
><uma.shankar@intel.com>; Maarten Lankhorst
><maarten.lankhorst@linux.intel.com>; Ville Syrjälä <ville.syrjala@linux.intel.com>
>Subject: [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output
>
>We recently added support for gen11's new "output csc" which can be used
>independently of the regular pipe CSC.  The idea is that this new output csc allows us
>to use a userspace-provided color transformation matrix at the same time we drive a
>YCBCR display mode requiring RGB->YUV conversion.  However when landing that
>support we only updated the color management code and overlooked that there was
>an additional atomic check in the modeset path (in intel_crtc_compute_config()) that
>still assumes we only have a single CSC unit to work with.  Let's update that check to
>only apply to pre-gen11 platforms and move it into the color management code to
>ensure it gets called on all commits, not just modesets.
>
>Also, if we're *only* using the output CSC and not the pipe CSC, there's no need to set
>crtc_state->csc_enable, so let's also not consider the output format when setting
>csc_enable on gen11+ platforms.

This looks good to me. Thanks for fixing this.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>Fixes: a91de580541c ("drm/i915/icl: Enable pipe output csc")
>Cc: Uma Shankar <uma.shankar@intel.com>
>Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>---
>It might also be worth moving the full->limited range RGB conversion from the pipe
>CSC to the output CSC on gen11 at some point.
>
> drivers/gpu/drm/i915/intel_color.c   | 16 +++++++++++++---
> drivers/gpu/drm/i915/intel_display.c | 12 ------------
> 2 files changed, 13 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>index da7a07d5ccea..b47e6d1aaaec 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -780,9 +780,10 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
> 	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> 		limited_color_range = crtc_state->limited_color_range;
>
>-	crtc_state->csc_enable =
>-		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
>-		crtc_state->base.ctm || limited_color_range;
>+	crtc_state->csc_enable = crtc_state->base.ctm || limited_color_range;
>+	if (INTEL_GEN(dev_priv) < 11)
>+		crtc_state->csc_enable |=
>+			crtc_state->output_format !=
>INTEL_OUTPUT_FORMAT_RGB;
>
> 	ret = intel_color_add_affected_planes(crtc_state);
> 	if (ret)
>@@ -822,6 +823,15 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
> 			crtc_state->csc_mode |= ICL_OUTPUT_CSC_ENABLE;
>
> 		crtc_state->csc_mode |= ICL_CSC_ENABLE;
>+	} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB &&
>+		   crtc_state->base.ctm) {
>+		/*
>+		 * There is only one pipe CSC unit per pipe on pre-gen11, and
>+		 * we need that for output conversion from RGB->YCBCR. So if
>+		 * CTM is already applied we can't support YCBCR output.
>+		 */
>+		DRM_DEBUG_KMS("YCBCR420 and CTM together are not
>possible\n");
>+		return -EINVAL;
> 	}
>
> 	return 0;
>diff --git a/drivers/gpu/drm/i915/intel_display.c
>b/drivers/gpu/drm/i915/intel_display.c
>index afa21daaae51..81bfdbd99092 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -6855,18 +6855,6 @@ static int intel_crtc_compute_config(struct intel_crtc
>*crtc,
> 		return -EINVAL;
> 	}
>
>-	if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
>-	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
>-	     pipe_config->base.ctm) {
>-		/*
>-		 * There is only one pipe CSC unit per pipe, and we need that
>-		 * for output conversion from RGB->YCBCR. So if CTM is already
>-		 * applied we can't support YCBCR420 output.
>-		 */
>-		DRM_DEBUG_KMS("YCBCR420 and CTM together are not
>possible\n");
>-		return -EINVAL;
>-	}
>-
> 	/*
> 	 * Pipe horizontal size must be even in:
> 	 * - DVO ganged mode
>--
>2.14.5

_______________________________________________
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: [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output
  2019-02-15 23:35 [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output Matt Roper
                   ` (2 preceding siblings ...)
  2019-02-18 14:28 ` [PATCH] " Shankar, Uma
@ 2019-02-18 20:28 ` Ville Syrjälä
  3 siblings, 0 replies; 5+ messages in thread
From: Ville Syrjälä @ 2019-02-18 20:28 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

On Fri, Feb 15, 2019 at 03:35:41PM -0800, Matt Roper wrote:
> We recently added support for gen11's new "output csc" which can be used
> independently of the regular pipe CSC.  The idea is that this new output
> csc allows us to use a userspace-provided color transformation matrix at
> the same time we drive a YCBCR display mode requiring RGB->YUV
> conversion.  However when landing that support we only updated the color
> management code and overlooked that there was an additional atomic check
> in the modeset path (in intel_crtc_compute_config()) that still assumes
> we only have a single CSC unit to work with.  Let's update that check to
> only apply to pre-gen11 platforms and move it into the color management
> code to ensure it gets called on all commits, not just modesets.
> 
> Also, if we're *only* using the output CSC and not the pipe CSC, there's
> no need to set crtc_state->csc_enable, so let's also not consider the
> output format when setting csc_enable on gen11+ platforms.
> 
> Fixes: a91de580541c ("drm/i915/icl: Enable pipe output csc")
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> It might also be worth moving the full->limited range RGB conversion
> from the pipe CSC to the output CSC on gen11 at some point.
> 
>  drivers/gpu/drm/i915/intel_color.c   | 16 +++++++++++++---
>  drivers/gpu/drm/i915/intel_display.c | 12 ------------
>  2 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index da7a07d5ccea..b47e6d1aaaec 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -780,9 +780,10 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>  	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
>  		limited_color_range = crtc_state->limited_color_range;
>  
> -	crtc_state->csc_enable =
> -		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
> -		crtc_state->base.ctm || limited_color_range;
> +	crtc_state->csc_enable = crtc_state->base.ctm || limited_color_range;
> +	if (INTEL_GEN(dev_priv) < 11)
> +		crtc_state->csc_enable |=
> +			crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB;
>  
>  	ret = intel_color_add_affected_planes(crtc_state);
>  	if (ret)
> @@ -822,6 +823,15 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>  			crtc_state->csc_mode |= ICL_OUTPUT_CSC_ENABLE;
>  
>  		crtc_state->csc_mode |= ICL_CSC_ENABLE;
> +	} else if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB &&
> +		   crtc_state->base.ctm) {
> +		/*
> +		 * There is only one pipe CSC unit per pipe on pre-gen11, and
> +		 * we need that for output conversion from RGB->YCBCR. So if
> +		 * CTM is already applied we can't support YCBCR output.
> +		 */
> +		DRM_DEBUG_KMS("YCBCR420 and CTM together are not possible\n");
> +		return -EINVAL;

BTW we have quite a few illegal combinations we're not currently
catching. I hacked together some checks for them here:
https://github.com/vsyrjala/linux/commit/0bc55aef05489a7e782bc14b7d5070eea52ce5ea
and then I'm lifting some of those restrictions for pre-glk here
https://github.com/vsyrjala/linux/commit/8db929ee8104c1ee640157d4ea8be15ea27451c5

Sadly on glk lost the flexibility to reorder the csc vs. a single LUT.
I think what we need to do there is to use the gamma LUT to do the
limited range compression, and for the YCbCr case we might just have
to load the software gamma LUT into the hw degamma LUT as best we can.
Otherwise the user may just get a totally unexpected fail when we
automagically decide that limited range/YCbCr output is required.

>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index afa21daaae51..81bfdbd99092 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6855,18 +6855,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> -	if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> -	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
> -	     pipe_config->base.ctm) {
> -		/*
> -		 * There is only one pipe CSC unit per pipe, and we need that
> -		 * for output conversion from RGB->YCBCR. So if CTM is already
> -		 * applied we can't support YCBCR420 output.
> -		 */
> -		DRM_DEBUG_KMS("YCBCR420 and CTM together are not possible\n");
> -		return -EINVAL;
> -	}
> -
>  	/*
>  	 * Pipe horizontal size must be even in:
>  	 * - DVO ganged mode
> -- 
> 2.14.5

-- 
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] 5+ messages in thread

end of thread, other threads:[~2019-02-18 20:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-15 23:35 [PATCH] drm/i915/icl: Fix checks for userspace ctm + ycbcr output Matt Roper
2019-02-16  0:44 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-02-16 11:34 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-02-18 14:28 ` [PATCH] " Shankar, Uma
2019-02-18 20:28 ` Ville Syrjälä

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.