[PATCH 0/3] drm/i915: color management atomic fixes

[PATCH 0/3] drm/i915: color management atomic fixes

[Maarten Lankhorst]

Some small fixes to color management.

crtc_state should be passed, a redundant check removed, and
color management updated during vblank evasion.

Maarten Lankhorst (3):
  drm/i915: Pass crtc_state to color management functions.
  drm/i915: Do not check crtc_state->active in intel_color_load_luts.
  drm/i915: Update color management during vblank evasion.

 drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
 drivers/gpu/drm/i915/intel_color.c   | 45 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |  4 ++--
 4 files changed, 46 insertions(+), 48 deletions(-)

-- 
2.1.0

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

[PATCH 1/3] drm/i915: Pass crtc_state to color management functions.

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
 drivers/gpu/drm/i915/intel_color.c   | 43 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  4 ++--
 4 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f6d71590bd7b..a93ba2903018 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -612,8 +612,8 @@ struct drm_i915_display_funcs {
 	/* display clock increase/decrease */
 	/* pll clock increase/decrease */
 
-	void (*load_csc_matrix)(struct drm_crtc *crtc);
-	void (*load_luts)(struct drm_crtc *crtc);
+	void (*load_csc_matrix)(struct drm_crtc_state *crtc_state);
+	void (*load_luts)(struct drm_crtc_state *crtc_state);
 };
 
 enum forcewake_domain_id {
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index aa0b20dcb834..9cffa638c351 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -92,10 +92,10 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
 }
 
 /* Set up the pipe CSC unit. */
-static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
+static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 {
+	struct drm_crtc *crtc = crtc_state->crtc;
 	struct drm_device *dev = crtc->dev;
-	struct drm_crtc_state *crtc_state = crtc->state;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int i, pipe = intel_crtc->pipe;
@@ -203,10 +203,10 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
 /*
  * Set up the pipe CSC unit on CherryView.
  */
-static void cherryview_load_csc_matrix(struct drm_crtc *crtc)
+static void cherryview_load_csc_matrix(struct drm_crtc_state *state)
 {
+	struct drm_crtc *crtc = state->crtc;
 	struct drm_device *dev = crtc->dev;
-	struct drm_crtc_state *state = crtc->state;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = to_intel_crtc(crtc)->pipe;
 	uint32_t mode;
@@ -252,13 +252,13 @@ static void cherryview_load_csc_matrix(struct drm_crtc *crtc)
 	I915_WRITE(CGM_PIPE_MODE(pipe), mode);
 }
 
-void intel_color_set_csc(struct drm_crtc *crtc)
+void intel_color_set_csc(struct drm_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc_state->crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (dev_priv->display.load_csc_matrix)
-		dev_priv->display.load_csc_matrix(crtc);
+		dev_priv->display.load_csc_matrix(crtc_state);
 }
 
 /* Loads the legacy palette/gamma unit for the CRTC. */
@@ -303,19 +303,20 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
 	}
 }
 
-static void i9xx_load_luts(struct drm_crtc *crtc)
+static void i9xx_load_luts(struct drm_crtc_state *crtc_state)
 {
-	i9xx_load_luts_internal(crtc, crtc->state->gamma_lut);
+	i9xx_load_luts_internal(crtc_state->crtc, crtc_state->gamma_lut);
 }
 
 /* Loads the legacy palette/gamma unit for the CRTC on Haswell. */
-static void haswell_load_luts(struct drm_crtc *crtc)
+static void haswell_load_luts(struct drm_crtc_state *crtc_state)
 {
+	struct drm_crtc *crtc = crtc_state->crtc;
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_crtc_state *intel_crtc_state =
-		to_intel_crtc_state(crtc->state);
+		to_intel_crtc_state(crtc_state);
 	bool reenable_ips = false;
 
 	/*
@@ -331,24 +332,24 @@ static void haswell_load_luts(struct drm_crtc *crtc)
 	intel_crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
 	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
 
-	i9xx_load_luts(crtc);
+	i9xx_load_luts(crtc_state);
 
 	if (reenable_ips)
 		hsw_enable_ips(intel_crtc);
 }
 
 /* Loads the palette/gamma unit for the CRTC on Broadwell+. */
-static void broadwell_load_luts(struct drm_crtc *crtc)
+static void broadwell_load_luts(struct drm_crtc_state *state)
 {
+	struct drm_crtc *crtc = state->crtc;
 	struct drm_device *dev = crtc->dev;
-	struct drm_crtc_state *state = crtc->state;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
 	enum pipe pipe = to_intel_crtc(crtc)->pipe;
 	uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size;
 
 	if (crtc_state_is_legacy(state)) {
-		haswell_load_luts(crtc);
+		haswell_load_luts(state);
 		return;
 	}
 
@@ -421,11 +422,11 @@ static void broadwell_load_luts(struct drm_crtc *crtc)
 }
 
 /* Loads the palette/gamma unit for the CRTC on CherryView. */
-static void cherryview_load_luts(struct drm_crtc *crtc)
+static void cherryview_load_luts(struct drm_crtc_state *state)
 {
+	struct drm_crtc *crtc = state->crtc;
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc_state *state = crtc->state;
 	enum pipe pipe = to_intel_crtc(crtc)->pipe;
 	struct drm_color_lut *lut;
 	uint32_t i, lut_size;
@@ -481,16 +482,16 @@ static void cherryview_load_luts(struct drm_crtc *crtc)
 	i9xx_load_luts_internal(crtc, NULL);
 }
 
-void intel_color_load_luts(struct drm_crtc *crtc)
+void intel_color_load_luts(struct drm_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = crtc_state->crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* The clocks have to be on to load the palette. */
-	if (!crtc->state->active)
+	if (!crtc_state->active)
 		return;
 
-	dev_priv->display.load_luts(crtc);
+	dev_priv->display.load_luts(crtc_state);
 }
 
 int intel_color_check(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 29aa64be1f03..4f913e5febf1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3223,7 +3223,7 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
 
 	if (HAS_DDI(dev))
-		intel_color_set_csc(&crtc->base);
+		intel_color_set_csc(&pipe_config->base);
 
 	/*
 	 * Update pipe size and adjust fitter if needed: the reason for this is
@@ -4723,6 +4723,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
+	struct intel_crtc_state *pipe_config =
+		to_intel_crtc_state(crtc->state);
 
 	if (WARN_ON(intel_crtc->active))
 		return;
@@ -4770,7 +4772,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_color_load_luts(crtc);
+	intel_color_load_luts(&pipe_config->base);
 
 	if (dev_priv->display.initial_watermarks != NULL)
 		dev_priv->display.initial_watermarks(intel_crtc->config);
@@ -4845,7 +4847,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	haswell_set_pipemisc(crtc);
 
-	intel_color_set_csc(crtc);
+	intel_color_set_csc(&pipe_config->base);
 
 	intel_crtc->active = true;
 
@@ -4874,7 +4876,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_color_load_luts(crtc);
+	intel_color_load_luts(&pipe_config->base);
 
 	intel_ddi_set_pipe_settings(crtc);
 	if (!intel_crtc->config->has_dsi_encoder)
@@ -6035,6 +6037,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
+	struct intel_crtc_state *pipe_config =
+		to_intel_crtc_state(crtc->state);
 	int pipe = intel_crtc->pipe;
 
 	if (WARN_ON(intel_crtc->active))
@@ -6079,7 +6083,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_pfit_enable(intel_crtc);
 
-	intel_color_load_luts(crtc);
+	intel_color_load_luts(&pipe_config->base);
 
 	intel_update_watermarks(crtc);
 	intel_enable_pipe(intel_crtc);
@@ -6106,6 +6110,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
+	struct intel_crtc_state *pipe_config =
+		to_intel_crtc_state(crtc->state);
 	int pipe = intel_crtc->pipe;
 
 	if (WARN_ON(intel_crtc->active))
@@ -6134,7 +6140,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_pfit_enable(intel_crtc);
 
-	intel_color_load_luts(crtc);
+	intel_color_load_luts(&pipe_config->base);
 
 	intel_update_watermarks(crtc);
 	intel_enable_pipe(intel_crtc);
@@ -13605,8 +13611,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 			 * a modeset as this will be done by
 			 * crtc_enable already.
 			 */
-			intel_color_set_csc(crtc);
-			intel_color_load_luts(crtc);
+			intel_color_set_csc(crtc->state);
+			intel_color_load_luts(crtc->state);
 		}
 
 		if (!modeset)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c87b4503435d..6ac46d921cde 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1669,7 +1669,7 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
 /* intel_color.c */
 void intel_color_init(struct drm_crtc *crtc);
 int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
-void intel_color_set_csc(struct drm_crtc *crtc);
-void intel_color_load_luts(struct drm_crtc *crtc);
+void intel_color_set_csc(struct drm_crtc_state *crtc_state);
+void intel_color_load_luts(struct drm_crtc_state *crtc_state);
 
 #endif /* __INTEL_DRV_H__ */
-- 
2.1.0

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

[PATCH 2/3] drm/i915: Do not check crtc_state->active in intel_color_load_luts.

[Maarten Lankhorst]

This is already tested by its callers.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 9cffa638c351..1b3f97449395 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -487,10 +487,6 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state)
 	struct drm_device *dev = crtc_state->crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/* The clocks have to be on to load the palette. */
-	if (!crtc_state->active)
-		return;
-
 	dev_priv->display.load_luts(crtc_state);
 }
 
-- 
2.1.0

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

[PATCH 3/3] drm/i915: Update color management during vblank evasion.

[Maarten Lankhorst]

Without this a vblank may occur between updating color management
and planes, which should be prevented.

intel_color_set_csc was called in update pipe config because the
handover from hardware may not have any csc set, which resulted
in a black screen. Because of this also update color management
during fastset.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4f913e5febf1..179461b45ce3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3222,9 +3222,6 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
 		      old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
 		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
 
-	if (HAS_DDI(dev))
-		intel_color_set_csc(&pipe_config->base);
-
 	/*
 	 * Update pipe size and adjust fitter if needed: the reason for this is
 	 * that in compute_mode_changes we check the native mode (not the pfit
@@ -13603,18 +13600,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 			dev_priv->display.crtc_enable(crtc);
 		}
 
-		if (!modeset &&
-		    crtc->state->active &&
-		    crtc->state->color_mgmt_changed) {
-			/*
-			 * Only update color management when not doing
-			 * a modeset as this will be done by
-			 * crtc_enable already.
-			 */
-			intel_color_set_csc(crtc->state);
-			intel_color_load_luts(crtc->state);
-		}
-
 		if (!modeset)
 			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
 
@@ -13933,6 +13918,16 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 	if (modeset)
 		return;
 
+	if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) {
+		/*
+		 * Only update color management when not doing
+		 * a modeset as this will be done by
+		 * crtc_enable already.
+		 */
+		intel_color_set_csc(crtc->state);
+		intel_color_load_luts(crtc->state);
+	}
+
 	if (to_intel_crtc_state(crtc->state)->update_pipe)
 		intel_update_pipe_config(intel_crtc, old_intel_state);
 	else if (INTEL_INFO(dev)->gen >= 9)
-- 
2.1.0

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

Re: [PATCH 3/3] drm/i915: Update color management during vblank evasion.

[Lionel Landwerlin]

On 30/03/16 16:16, Maarten Lankhorst wrote:
> Without this a vblank may occur between updating color management
> and planes, which should be prevented.
>
> intel_color_set_csc was called in update pipe config because the
> handover from hardware may not have any csc set, which resulted
> in a black screen. Because of this also update color management
> during fastset.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

> ---
>   drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++---------------
>   1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4f913e5febf1..179461b45ce3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3222,9 +3222,6 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
>   		      old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
>   		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>   
> -	if (HAS_DDI(dev))
> -		intel_color_set_csc(&pipe_config->base);
> -
>   	/*
>   	 * Update pipe size and adjust fitter if needed: the reason for this is
>   	 * that in compute_mode_changes we check the native mode (not the pfit
> @@ -13603,18 +13600,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>   			dev_priv->display.crtc_enable(crtc);
>   		}
>   
> -		if (!modeset &&
> -		    crtc->state->active &&
> -		    crtc->state->color_mgmt_changed) {
> -			/*
> -			 * Only update color management when not doing
> -			 * a modeset as this will be done by
> -			 * crtc_enable already.
> -			 */
> -			intel_color_set_csc(crtc->state);
> -			intel_color_load_luts(crtc->state);
> -		}
> -
>   		if (!modeset)
>   			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
>   
> @@ -13933,6 +13918,16 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
>   	if (modeset)
>   		return;
>   
> +	if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) {
> +		/*
> +		 * Only update color management when not doing
> +		 * a modeset as this will be done by
> +		 * crtc_enable already.
> +		 */
I guess we can drop this comment.
> +		intel_color_set_csc(crtc->state);
> +		intel_color_load_luts(crtc->state);
> +	}
> +
>   	if (to_intel_crtc_state(crtc->state)->update_pipe)
>   		intel_update_pipe_config(intel_crtc, old_intel_state);
>   	else if (INTEL_INFO(dev)->gen >= 9)

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

Re: [PATCH 2/3] drm/i915: Do not check crtc_state->active in intel_color_load_luts.

[Lionel Landwerlin]

On 30/03/16 16:16, Maarten Lankhorst wrote:
> This is already tested by its callers.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_color.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 9cffa638c351..1b3f97449395 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -487,10 +487,6 @@ void intel_color_load_luts(struct drm_crtc_state *crtc_state)
>   	struct drm_device *dev = crtc_state->crtc->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   
> -	/* The clocks have to be on to load the palette. */
> -	if (!crtc_state->active)
> -		return;
> -
>   	dev_priv->display.load_luts(crtc_state);
>   }
>   

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

Re: [PATCH 1/3] drm/i915: Pass crtc_state to color management functions.

[Lionel Landwerlin]

On 30/03/16 16:16, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  4 ++--
>   drivers/gpu/drm/i915/intel_color.c   | 43 ++++++++++++++++++------------------
>   drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++-------
>   drivers/gpu/drm/i915/intel_drv.h     |  4 ++--
>   4 files changed, 40 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f6d71590bd7b..a93ba2903018 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -612,8 +612,8 @@ struct drm_i915_display_funcs {
>   	/* display clock increase/decrease */
>   	/* pll clock increase/decrease */
>   
> -	void (*load_csc_matrix)(struct drm_crtc *crtc);
> -	void (*load_luts)(struct drm_crtc *crtc);
> +	void (*load_csc_matrix)(struct drm_crtc_state *crtc_state);
> +	void (*load_luts)(struct drm_crtc_state *crtc_state);
>   };
>   
>   enum forcewake_domain_id {
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index aa0b20dcb834..9cffa638c351 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -92,10 +92,10 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
>   }
>   
>   /* Set up the pipe CSC unit. */
> -static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
> +static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
>   {
> +	struct drm_crtc *crtc = crtc_state->crtc;
>   	struct drm_device *dev = crtc->dev;
> -	struct drm_crtc_state *crtc_state = crtc->state;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	int i, pipe = intel_crtc->pipe;
> @@ -203,10 +203,10 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
>   /*
>    * Set up the pipe CSC unit on CherryView.
>    */
> -static void cherryview_load_csc_matrix(struct drm_crtc *crtc)
> +static void cherryview_load_csc_matrix(struct drm_crtc_state *state)
>   {
> +	struct drm_crtc *crtc = state->crtc;
>   	struct drm_device *dev = crtc->dev;
> -	struct drm_crtc_state *state = crtc->state;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	int pipe = to_intel_crtc(crtc)->pipe;
>   	uint32_t mode;
> @@ -252,13 +252,13 @@ static void cherryview_load_csc_matrix(struct drm_crtc *crtc)
>   	I915_WRITE(CGM_PIPE_MODE(pipe), mode);
>   }
>   
> -void intel_color_set_csc(struct drm_crtc *crtc)
> +void intel_color_set_csc(struct drm_crtc_state *crtc_state)
>   {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc_state->crtc->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   
>   	if (dev_priv->display.load_csc_matrix)
> -		dev_priv->display.load_csc_matrix(crtc);
> +		dev_priv->display.load_csc_matrix(crtc_state);
>   }
>   
>   /* Loads the legacy palette/gamma unit for the CRTC. */
> @@ -303,19 +303,20 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
>   	}
>   }
>   
> -static void i9xx_load_luts(struct drm_crtc *crtc)
> +static void i9xx_load_luts(struct drm_crtc_state *crtc_state)
>   {
> -	i9xx_load_luts_internal(crtc, crtc->state->gamma_lut);
> +	i9xx_load_luts_internal(crtc_state->crtc, crtc_state->gamma_lut);
>   }
>   
>   /* Loads the legacy palette/gamma unit for the CRTC on Haswell. */
> -static void haswell_load_luts(struct drm_crtc *crtc)
> +static void haswell_load_luts(struct drm_crtc_state *crtc_state)
>   {
> +	struct drm_crtc *crtc = crtc_state->crtc;
>   	struct drm_device *dev = crtc->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	struct intel_crtc_state *intel_crtc_state =
> -		to_intel_crtc_state(crtc->state);
> +		to_intel_crtc_state(crtc_state);
>   	bool reenable_ips = false;
>   
>   	/*
> @@ -331,24 +332,24 @@ static void haswell_load_luts(struct drm_crtc *crtc)
>   	intel_crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
>   	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
>   
> -	i9xx_load_luts(crtc);
> +	i9xx_load_luts(crtc_state);
>   
>   	if (reenable_ips)
>   		hsw_enable_ips(intel_crtc);
>   }
>   
>   /* Loads the palette/gamma unit for the CRTC on Broadwell+. */
> -static void broadwell_load_luts(struct drm_crtc *crtc)
> +static void broadwell_load_luts(struct drm_crtc_state *state)
>   {
> +	struct drm_crtc *crtc = state->crtc;
>   	struct drm_device *dev = crtc->dev;
> -	struct drm_crtc_state *state = crtc->state;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
>   	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>   	uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size;
>   
>   	if (crtc_state_is_legacy(state)) {
> -		haswell_load_luts(crtc);
> +		haswell_load_luts(state);
>   		return;
>   	}
>   
> @@ -421,11 +422,11 @@ static void broadwell_load_luts(struct drm_crtc *crtc)
>   }
>   
>   /* Loads the palette/gamma unit for the CRTC on CherryView. */
> -static void cherryview_load_luts(struct drm_crtc *crtc)
> +static void cherryview_load_luts(struct drm_crtc_state *state)
>   {
> +	struct drm_crtc *crtc = state->crtc;
>   	struct drm_device *dev = crtc->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_crtc_state *state = crtc->state;
>   	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>   	struct drm_color_lut *lut;
>   	uint32_t i, lut_size;
> @@ -481,16 +482,16 @@ static void cherryview_load_luts(struct drm_crtc *crtc)
>   	i9xx_load_luts_internal(crtc, NULL);
>   }
>   
> -void intel_color_load_luts(struct drm_crtc *crtc)
> +void intel_color_load_luts(struct drm_crtc_state *crtc_state)
>   {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc_state->crtc->dev;
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   
>   	/* The clocks have to be on to load the palette. */
> -	if (!crtc->state->active)
> +	if (!crtc_state->active)
>   		return;
>   
> -	dev_priv->display.load_luts(crtc);
> +	dev_priv->display.load_luts(crtc_state);
>   }
>   
>   int intel_color_check(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 29aa64be1f03..4f913e5febf1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3223,7 +3223,7 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
>   		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>   
>   	if (HAS_DDI(dev))
> -		intel_color_set_csc(&crtc->base);
> +		intel_color_set_csc(&pipe_config->base);
>   
>   	/*
>   	 * Update pipe size and adjust fitter if needed: the reason for this is
> @@ -4723,6 +4723,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	struct intel_encoder *encoder;
>   	int pipe = intel_crtc->pipe;
> +	struct intel_crtc_state *pipe_config =
> +		to_intel_crtc_state(crtc->state);
>   
>   	if (WARN_ON(intel_crtc->active))
>   		return;
> @@ -4770,7 +4772,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>   	 * On ILK+ LUT must be loaded before the pipe is running but with
>   	 * clocks enabled
>   	 */
> -	intel_color_load_luts(crtc);
> +	intel_color_load_luts(&pipe_config->base);
>   
>   	if (dev_priv->display.initial_watermarks != NULL)
>   		dev_priv->display.initial_watermarks(intel_crtc->config);
> @@ -4845,7 +4847,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>   
>   	haswell_set_pipemisc(crtc);
>   
> -	intel_color_set_csc(crtc);
> +	intel_color_set_csc(&pipe_config->base);
>   
>   	intel_crtc->active = true;
>   
> @@ -4874,7 +4876,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>   	 * On ILK+ LUT must be loaded before the pipe is running but with
>   	 * clocks enabled
>   	 */
> -	intel_color_load_luts(crtc);
> +	intel_color_load_luts(&pipe_config->base);
>   
>   	intel_ddi_set_pipe_settings(crtc);
>   	if (!intel_crtc->config->has_dsi_encoder)
> @@ -6035,6 +6037,8 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>   	struct drm_i915_private *dev_priv = to_i915(dev);
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	struct intel_encoder *encoder;
> +	struct intel_crtc_state *pipe_config =
> +		to_intel_crtc_state(crtc->state);
>   	int pipe = intel_crtc->pipe;
>   
>   	if (WARN_ON(intel_crtc->active))
> @@ -6079,7 +6083,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>   
>   	i9xx_pfit_enable(intel_crtc);
>   
> -	intel_color_load_luts(crtc);
> +	intel_color_load_luts(&pipe_config->base);
>   
>   	intel_update_watermarks(crtc);
>   	intel_enable_pipe(intel_crtc);
> @@ -6106,6 +6110,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>   	struct drm_i915_private *dev_priv = to_i915(dev);
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	struct intel_encoder *encoder;
> +	struct intel_crtc_state *pipe_config =
> +		to_intel_crtc_state(crtc->state);
>   	int pipe = intel_crtc->pipe;
>   
>   	if (WARN_ON(intel_crtc->active))
> @@ -6134,7 +6140,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>   
>   	i9xx_pfit_enable(intel_crtc);
>   
> -	intel_color_load_luts(crtc);
> +	intel_color_load_luts(&pipe_config->base);
>   
>   	intel_update_watermarks(crtc);
>   	intel_enable_pipe(intel_crtc);
> @@ -13605,8 +13611,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>   			 * a modeset as this will be done by
>   			 * crtc_enable already.
>   			 */
> -			intel_color_set_csc(crtc);
> -			intel_color_load_luts(crtc);
> +			intel_color_set_csc(crtc->state);
> +			intel_color_load_luts(crtc->state);
>   		}
>   
>   		if (!modeset)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c87b4503435d..6ac46d921cde 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1669,7 +1669,7 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>   /* intel_color.c */
>   void intel_color_init(struct drm_crtc *crtc);
>   int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
> -void intel_color_set_csc(struct drm_crtc *crtc);
> -void intel_color_load_luts(struct drm_crtc *crtc);
> +void intel_color_set_csc(struct drm_crtc_state *crtc_state);
> +void intel_color_load_luts(struct drm_crtc_state *crtc_state);
>   
>   #endif /* __INTEL_DRV_H__ */

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

Re: [PATCH 3/3] drm/i915: Update color management during vblank evasion.

[Lionel Landwerlin]

On 30/03/16 17:41, Lionel Landwerlin wrote:
> On 30/03/16 16:16, Maarten Lankhorst wrote:
>> Without this a vblank may occur between updating color management
>> and planes, which should be prevented.
>>
>> intel_color_set_csc was called in update pipe config because the
>> handover from hardware may not have any csc set, which resulted
>> in a black screen. Because of this also update color management
>> during fastset.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++---------------
>>   1 file changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 4f913e5febf1..179461b45ce3 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3222,9 +3222,6 @@ static void intel_update_pipe_config(struct 
>> intel_crtc *crtc,
>>                 old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
>>                 pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>>   -    if (HAS_DDI(dev))
>> -        intel_color_set_csc(&pipe_config->base);
>> -
>>       /*
>>        * Update pipe size and adjust fitter if needed: the reason for 
>> this is
>>        * that in compute_mode_changes we check the native mode (not 
>> the pfit
>> @@ -13603,18 +13600,6 @@ static int intel_atomic_commit(struct 
>> drm_device *dev,
>>               dev_priv->display.crtc_enable(crtc);
>>           }
>>   -        if (!modeset &&
>> -            crtc->state->active &&
>> -            crtc->state->color_mgmt_changed) {
>> -            /*
>> -             * Only update color management when not doing
>> -             * a modeset as this will be done by
>> -             * crtc_enable already.
>> -             */
>> -            intel_color_set_csc(crtc->state);
>> -            intel_color_load_luts(crtc->state);
>> -        }
>> -
>>           if (!modeset)
>> intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
>>   @@ -13933,6 +13918,16 @@ static void intel_begin_crtc_commit(struct 
>> drm_crtc *crtc,
>>       if (modeset)
>>           return;
>>   +    if (crtc->state->color_mgmt_changed || 
>> to_intel_crtc_state(crtc->state)->update_pipe) {
>> +        /*
>> +         * Only update color management when not doing
>> +         * a modeset as this will be done by
>> +         * crtc_enable already.
>> +         */
> I guess we can drop this comment.
>> + intel_color_set_csc(crtc->state);
>> +        intel_color_load_luts(crtc->state);
>> +    }
>> +
>>       if (to_intel_crtc_state(crtc->state)->update_pipe)
>>           intel_update_pipe_config(intel_crtc, old_intel_state);
>>       else if (INTEL_INFO(dev)->gen >= 9)
>
> _______________________________________________
> 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

Re: [PATCH 3/3] drm/i915: Update color management during vblank evasion.

[Lionel Landwerlin]

On 30/03/16 17:41, Lionel Landwerlin wrote:
> On 30/03/16 16:16, Maarten Lankhorst wrote:
>> Without this a vblank may occur between updating color management
>> and planes, which should be prevented.
>>
>> intel_color_set_csc was called in update pipe config because the
>> handover from hardware may not have any csc set, which resulted
>> in a black screen. Because of this also update color management
>> during fastset.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 25 ++++++++++---------------
>>   1 file changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c 
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 4f913e5febf1..179461b45ce3 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3222,9 +3222,6 @@ static void intel_update_pipe_config(struct 
>> intel_crtc *crtc,
>>                 old_crtc_state->pipe_src_w, old_crtc_state->pipe_src_h,
>>                 pipe_config->pipe_src_w, pipe_config->pipe_src_h);
>>   -    if (HAS_DDI(dev))
>> -        intel_color_set_csc(&pipe_config->base);
>> -
>>       /*
>>        * Update pipe size and adjust fitter if needed: the reason for 
>> this is
>>        * that in compute_mode_changes we check the native mode (not 
>> the pfit
>> @@ -13603,18 +13600,6 @@ static int intel_atomic_commit(struct 
>> drm_device *dev,
>>               dev_priv->display.crtc_enable(crtc);
>>           }
>>   -        if (!modeset &&
>> -            crtc->state->active &&
>> -            crtc->state->color_mgmt_changed) {
>> -            /*
>> -             * Only update color management when not doing
>> -             * a modeset as this will be done by
>> -             * crtc_enable already.
>> -             */
>> -            intel_color_set_csc(crtc->state);
>> -            intel_color_load_luts(crtc->state);
>> -        }
>> -
>>           if (!modeset)
>> intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
>>   @@ -13933,6 +13918,16 @@ static void intel_begin_crtc_commit(struct 
>> drm_crtc *crtc,
>>       if (modeset)
>>           return;
>>   +    if (crtc->state->color_mgmt_changed || 
>> to_intel_crtc_state(crtc->state)->update_pipe) {
>> +        /*
>> +         * Only update color management when not doing
>> +         * a modeset as this will be done by
>> +         * crtc_enable already.
>> +         */
> I guess we can drop this comment.
>> + intel_color_set_csc(crtc->state);
>> +        intel_color_load_luts(crtc->state);
>> +    }
>> +
>>       if (to_intel_crtc_state(crtc->state)->update_pipe)
>>           intel_update_pipe_config(intel_crtc, old_intel_state);
>>       else if (INTEL_INFO(dev)->gen >= 9)
>
> _______________________________________________
> 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