>-----Original Message-----
>From: Roper, Matthew D
>Sent: Saturday, February 16, 2019 5:06 AM
>To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>Cc: Roper, Matthew D <matthew.d.roper@xxxxxxxxx>; Shankar, Uma
><uma.shankar@xxxxxxxxx>; Maarten Lankhorst
><maarten.lankhorst@xxxxxxxxxxxxxxx>; Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>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@xxxxxxxxx>
>Fixes: a91de580541c ("drm/i915/icl: Enable pipe output csc")
>Cc: Uma Shankar <uma.shankar@xxxxxxxxx>
>Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
>---
>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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
