intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Drew Davenport <ddavenport@chromium.org>
Subject: [Intel-gfx] [PATCH 2/6] drm/i915: Move CSC load back into .color_commit_arm() when PSR is enabled on skl/glk
Date: Mon, 20 Mar 2023 11:54:34 +0200	[thread overview]
Message-ID: <20230320095438.17328-3-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20230320095438.17328-1-ville.syrjala@linux.intel.com>

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

SKL/GLK CSC unit suffers from a nasty issue where a CSC
coeff/offset register read or write between DC5 exit and
PSR exit will undo the CSC arming performed by DMC, and
then during PSR exit the hardware will latch zeroes into
the active CSC registers. This causes any plane going
through the CSC to output all black.

We can sidestep the issue by making sure the PSR exit has
already actually happened before we touch the CSC coeff/offset
registers. Easiest way to guarantee that is to just move the
CSC programming back into the .color_commir_arm() as we force
a PSR exit (and crucially wait for it to actually happen)
prior to touching the arming registers.

When PSR (and thus also DC states) are disabled we don't
have anything to worry about, so we can keep using the
more optional _noarm() hook for writing the CSC registers.

Cc: Manasi Navare <navaremanasi@google.com>
Cc: Drew Davenport <ddavenport@chromium.org>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jouni Högander <jouni.hogander@intel.com>
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/8283
Fixes: d13dde449580 ("drm/i915: Split pipe+output CSC programming to noarm+arm pair")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 23 ++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 7c8f40cd989e..37fb5a7bc8d8 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -617,6 +617,22 @@ static void icl_color_commit_noarm(const struct intel_crtc_state *crtc_state)
 	icl_load_csc_matrix(crtc_state);
 }
 
+static void skl_color_commit_noarm(const struct intel_crtc_state *crtc_state)
+{
+	/*
+	 * Possibly related to display WA #1184, SKL CSC loses the latched
+	 * CSC coeff/offset register values if the CSC registers are disarmed
+	 * between DC5 exit and PSR exit. This will cause the plane(s) to
+	 * output all black (until CSC_MODE is rearmed and properly latched).
+	 * Once PSR exit (and proper register latching) has occurred the
+	 * danger is over. Thus when PSR is enabled the CSC coeff/offset
+	 * register programming will be peformed from skl_color_commit_arm()
+	 * which is called after PSR exit.
+	 */
+	if (!crtc_state->has_psr)
+		ilk_load_csc_matrix(crtc_state);
+}
+
 static void ilk_color_commit_noarm(const struct intel_crtc_state *crtc_state)
 {
 	ilk_load_csc_matrix(crtc_state);
@@ -659,6 +675,9 @@ static void skl_color_commit_arm(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	u32 val = 0;
 
+	if (crtc_state->has_psr)
+		ilk_load_csc_matrix(crtc_state);
+
 	/*
 	 * We don't (yet) allow userspace to control the pipe background color,
 	 * so force it to black, but apply pipe gamma and CSC appropriately
@@ -3103,7 +3122,7 @@ static const struct intel_color_funcs icl_color_funcs = {
 
 static const struct intel_color_funcs glk_color_funcs = {
 	.color_check = glk_color_check,
-	.color_commit_noarm = ilk_color_commit_noarm,
+	.color_commit_noarm = skl_color_commit_noarm,
 	.color_commit_arm = skl_color_commit_arm,
 	.load_luts = glk_load_luts,
 	.read_luts = glk_read_luts,
@@ -3112,7 +3131,7 @@ static const struct intel_color_funcs glk_color_funcs = {
 
 static const struct intel_color_funcs skl_color_funcs = {
 	.color_check = ivb_color_check,
-	.color_commit_noarm = ilk_color_commit_noarm,
+	.color_commit_noarm = skl_color_commit_noarm,
 	.color_commit_arm = skl_color_commit_arm,
 	.load_luts = bdw_load_luts,
 	.read_luts = bdw_read_luts,
-- 
2.39.2


  parent reply	other threads:[~2023-03-20  9:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-20  9:54 [Intel-gfx] [PATCH 0/6] drm/i915: Fix various issues with noarm register writes Ville Syrjala
2023-03-20  9:54 ` [Intel-gfx] [PATCH 1/6] drm/i915: Split icl_color_commit_noarm() from skl_color_commit_noarm() Ville Syrjala
2023-03-20  9:54 ` Ville Syrjala [this message]
2023-03-20  9:54 ` [Intel-gfx] [PATCH 3/6] drm/i915: Add a .color_post_update() hook Ville Syrjala
2023-03-20  9:54 ` [Intel-gfx] [PATCH 4/6] drm/i915: Workaround ICL CSC_MODE sticky arming Ville Syrjala
2023-03-20  9:54 ` [Intel-gfx] [PATCH 5/6] drm/i915: Disable DC states for all commits Ville Syrjala
2023-03-20 11:51   ` Imre Deak
2023-03-20 18:24     ` Ville Syrjälä
2023-03-20 18:35   ` [Intel-gfx] [PATCH v2 " Ville Syrjala
2023-03-20  9:54 ` [Intel-gfx] [PATCH 6/6] drm/i915/psr: Define more PSR mask bits Ville Syrjala
2023-03-20 12:05   ` Imre Deak
2023-03-20 12:23     ` Ville Syrjälä
2023-03-21 20:45   ` Ville Syrjälä
2023-03-20 18:01 ` [Intel-gfx] ✗ Fi.CI.BUILD: warning for drm/i915: Fix various issues with noarm register writes Patchwork
2023-03-20 18:02 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: " Patchwork
2023-03-20 18:02 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-03-20 18:02 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2023-03-20 18:23 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2023-03-21  6:07 ` [Intel-gfx] ✗ Fi.CI.BUILD: warning for drm/i915: Fix various issues with noarm register writes (rev2) Patchwork
2023-03-21  6:07 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: " Patchwork
2023-03-21  6:07 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-03-21  6:33 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-03-21  9:59 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230320095438.17328-3-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=ddavenport@chromium.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).