[PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch

[PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch

[Vidya Srinivas]

From: Chandra Konduru <chandra.konduru@intel.com>

Display WA #0827:
Switching the plane format from NV12 to RGB and leaving system idle results
in display underrun and corruption. WA: Set the bit 15 & bit 19 to 1b
in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane is enabled.

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f36023..c4af05e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3822,6 +3822,9 @@ enum {
 #define _CLKGATE_DIS_PSL_A		0x46520
 #define _CLKGATE_DIS_PSL_B		0x46524
 #define _CLKGATE_DIS_PSL_C		0x46528
+#define DUPS1_GATING_DIS	(1 << 15)
+#define DUPS2_GATING_DIS	(1 << 19)
+#define DUPS3_GATING_DIS	(1 << 23)
 #define   DPF_GATING_DIS		(1 << 10)
 #define   DPF_RAM_GATING_DIS		(1 << 9)
 #define   DPFR_GATING_DIS		(1 << 8)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 551c970..94faf3e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5495,6 +5495,20 @@ static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
 	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);
 }
 
+static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
+	int pipe, int enable)
+{
+	if (pipe == PIPE_A || pipe == PIPE_B) {
+		if (enable)
+			I915_WRITE(CLKGATE_DIS_PSL(pipe),
+				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
+		else
+			I915_WRITE(CLKGATE_DIS_PSL(pipe),
+			I915_READ(CLKGATE_DIS_PSL(pipe)) &
+			~(DUPS1_GATING_DIS|DUPS2_GATING_DIS));
+	}
+}
+
 static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 				struct drm_atomic_state *old_state)
 {
@@ -5599,6 +5613,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
 		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
 		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
 	}
+	skl_wa_clkgate(dev_priv, pipe, 1);
 }
 
 static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
@@ -5709,6 +5724,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
 		intel_ddi_disable_pipe_clock(intel_crtc->config);
 
 	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
+	skl_wa_clkgate(dev_priv, intel_crtc->pipe, 0);
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
-- 
1.9.1

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

[PATCH] drm/i915: Enable YUV to RGB for Gen10 in Plane Ctrl Reg

[Vidya Srinivas]

If the fb format is YUV, enable the plane CSC mode bits
for the conversion.

Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 6 ++++++
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 492a1b8..6db7d1a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6384,6 +6384,12 @@ enum {
 #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
 #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30)
 #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23)
+#define PLANE_COLOR_CSC_MASK	(0x7 << 17)
+#define PLANE_COLOR_CSC_MODE_BYPASS	(0 << 17)
+#define PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709	(1 << 17)
+#define PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709	(2 << 17)
+#define PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020	(3 << 17)
+#define PLANE_COLOR_CSC_MODE_RGB709_TO_RGB2020	(4 << 17)
 #define   PLANE_COLOR_PLANE_GAMMA_DISABLE	(1 << 13)
 #define   PLANE_COLOR_ALPHA_MASK		(0x3 << 4)
 #define   PLANE_COLOR_ALPHA_DISABLE		(0 << 4)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 637b7d0..a6ec427 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3539,6 +3539,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	plane_color_ctl |= PLANE_COLOR_PIPE_CSC_ENABLE;
 	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
 	plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
+	if (intel_format_is_yuv(fb->format->format))
+		plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
 
 	return plane_color_ctl;
 }
-- 
2.7.4

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

[PATCH] drm/i915: NV12 changes for Gen10

[Vidya Srinivas]

Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c  | 5 ++---
 drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
 drivers/gpu/drm/i915/intel_sprite.c  | 4 ++++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index ecba7c7..ca213bc 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -327,14 +327,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
 		}
 
 		/* set scaler mode */
-		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
+		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv) ||
+			IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
 			plane_state && plane_state->base.fb &&
 			plane_state->base.fb->format->format ==
 			DRM_FORMAT_NV12) {
 			scaler_state->scalers[*scaler_id].mode =
 				PS_SCALER_MODE_NV12;
-		} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
-			scaler_state->scalers[*scaler_id].mode = 0;
 		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
 			/*
 			 * when only 1 scaler is in use on either pipe A or B,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index df6b11a..115e0ea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13200,7 +13200,11 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 			((pipe == PIPE_A || pipe == PIPE_B))) {
 			intel_primary_formats = nv12_primary_formats;
 			num_formats = ARRAY_SIZE(nv12_primary_formats);
+		} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+			intel_primary_formats = nv12_primary_formats;
+			num_formats = ARRAY_SIZE(nv12_primary_formats);
 		} else {
+
 			intel_primary_formats = skl_primary_formats;
 			num_formats = ARRAY_SIZE(skl_primary_formats);
 		}
@@ -14006,7 +14010,8 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 		}
 		break;
 	case DRM_FORMAT_NV12:
-		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv)) {
+		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv) &&
+			!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
 			DRM_DEBUG_KMS("unsupported pixel format: %s\n",
 		      drm_get_format_name(mode_cmd->pixel_format,
 				&format_name));
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 1d35a18..35dcba0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1340,6 +1340,10 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 			(pipe == PIPE_A || pipe == PIPE_B) && plane == 0) {
 			plane_formats = nv12_plane_formats;
 			num_plane_formats = ARRAY_SIZE(nv12_plane_formats);
+		} else if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
+				   plane == 0) {
+			plane_formats = nv12_plane_formats;
+			num_plane_formats = ARRAY_SIZE(nv12_plane_formats);
 		} else {
 			plane_formats = skl_plane_formats;
 			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
-- 
2.7.4

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

Re: [PATCH] drm/i915: NV12 changes for Gen10

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 4:36 PM, Vidya Srinivas wrote:
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
No need for any commit message :P ?
> ---
>   drivers/gpu/drm/i915/intel_atomic.c  | 5 ++---
>   drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
>   drivers/gpu/drm/i915/intel_sprite.c  | 4 ++++
>   3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index ecba7c7..ca213bc 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -327,14 +327,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>   		}
>   
>   		/* set scaler mode */
> -		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> +		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv) ||
> +			IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
Please align the conditions to above line, below existing IS_BXT
>   			plane_state && plane_state->base.fb &&
>   			plane_state->base.fb->format->format ==
>   			DRM_FORMAT_NV12) {
>   			scaler_state->scalers[*scaler_id].mode =
>   				PS_SCALER_MODE_NV12;
> -		} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> -			scaler_state->scalers[*scaler_id].mode = 0;
>   		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
>   			/*
>   			 * when only 1 scaler is in use on either pipe A or B,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index df6b11a..115e0ea 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13200,7 +13200,11 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
>   			((pipe == PIPE_A || pipe == PIPE_B))) {
>   			intel_primary_formats = nv12_primary_formats;
>   			num_formats = ARRAY_SIZE(nv12_primary_formats);
> +		} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
> +			intel_primary_formats = nv12_primary_formats;
> +			num_formats = ARRAY_SIZE(nv12_primary_formats);
I can see these same lines in the above condition, can we simply add 
this (|| IS_GLK || IS_CNL) condition in above ? Or not ?
>   		} else {
> +
Extra line
>   			intel_primary_formats = skl_primary_formats;
>   			num_formats = ARRAY_SIZE(skl_primary_formats);
>   		}
> @@ -14006,7 +14010,8 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>   		}
>   		break;
>   	case DRM_FORMAT_NV12:
> -		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv)) {
> +		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv) &&
> +			!IS_GEMINILAKE(dev_priv) && !IS_CANNONLAKE(dev_priv)) {
Alignment
>   			DRM_DEBUG_KMS("unsupported pixel format: %s\n",
>   		      drm_get_format_name(mode_cmd->pixel_format,
>   				&format_name));
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 1d35a18..35dcba0 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1340,6 +1340,10 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>   			(pipe == PIPE_A || pipe == PIPE_B) && plane == 0) {
>   			plane_formats = nv12_plane_formats;
>   			num_plane_formats = ARRAY_SIZE(nv12_plane_formats);
> +		} else if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +				   plane == 0) {
> +			plane_formats = nv12_plane_formats;
> +			num_plane_formats = ARRAY_SIZE(nv12_plane_formats);
Same as above, can this merge into condition just above this if ?

- Shashank
>   		} else {
>   			plane_formats = skl_plane_formats;
>   			num_plane_formats = ARRAY_SIZE(skl_plane_formats);

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

Re: [PATCH] drm/i915: NV12 changes for Gen10

[Srinivas, Vidya]

Apologies. This patch was pushed wrongly. Not a part of the 16 patch series of NV12.

Regards
Vidya

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 6:17 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [PATCH] drm/i915: NV12 changes for Gen10
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 4:36 PM, Vidya Srinivas wrote:
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> No need for any commit message :P ?
> > ---
> >   drivers/gpu/drm/i915/intel_atomic.c  | 5 ++---
> >   drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
> >   drivers/gpu/drm/i915/intel_sprite.c  | 4 ++++
> >   3 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > b/drivers/gpu/drm/i915/intel_atomic.c
> > index ecba7c7..ca213bc 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -327,14 +327,13 @@ int intel_atomic_setup_scalers(struct
> drm_i915_private *dev_priv,
> >   		}
> >
> >   		/* set scaler mode */
> > -		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> > +		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv) ||
> > +			IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) &&
> Please align the conditions to above line, below existing IS_BXT
> >   			plane_state && plane_state->base.fb &&
> >   			plane_state->base.fb->format->format ==
> >   			DRM_FORMAT_NV12) {
> >   			scaler_state->scalers[*scaler_id].mode =
> >   				PS_SCALER_MODE_NV12;
> > -		} else if (IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) {
> > -			scaler_state->scalers[*scaler_id].mode = 0;
> >   		} else if (num_scalers_need == 1 && intel_crtc->pipe !=
> PIPE_C) {
> >   			/*
> >   			 * when only 1 scaler is in use on either pipe A or B,
> diff --git
> > a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index df6b11a..115e0ea 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13200,7 +13200,11 @@ intel_primary_plane_create(struct
> drm_i915_private *dev_priv, enum pipe pipe)
> >   			((pipe == PIPE_A || pipe == PIPE_B))) {
> >   			intel_primary_formats = nv12_primary_formats;
> >   			num_formats =
> ARRAY_SIZE(nv12_primary_formats);
> > +		} else if (IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) {
> > +			intel_primary_formats = nv12_primary_formats;
> > +			num_formats =
> ARRAY_SIZE(nv12_primary_formats);
> I can see these same lines in the above condition, can we simply add this (||
> IS_GLK || IS_CNL) condition in above ? Or not ?
> >   		} else {
> > +
> Extra line
> >   			intel_primary_formats = skl_primary_formats;
> >   			num_formats = ARRAY_SIZE(skl_primary_formats);
> >   		}
> > @@ -14006,7 +14010,8 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> >   		}
> >   		break;
> >   	case DRM_FORMAT_NV12:
> > -		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv)) {
> > +		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv) &&
> > +			!IS_GEMINILAKE(dev_priv) &&
> !IS_CANNONLAKE(dev_priv)) {
> Alignment
> >   			DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> >   		      drm_get_format_name(mode_cmd->pixel_format,
> >   				&format_name));
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 1d35a18..35dcba0 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1340,6 +1340,10 @@ intel_sprite_plane_create(struct
> drm_i915_private *dev_priv,
> >   			(pipe == PIPE_A || pipe == PIPE_B) && plane == 0) {
> >   			plane_formats = nv12_plane_formats;
> >   			num_plane_formats =
> ARRAY_SIZE(nv12_plane_formats);
> > +		} else if ((IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) &&
> > +				   plane == 0) {
> > +			plane_formats = nv12_plane_formats;
> > +			num_plane_formats =
> ARRAY_SIZE(nv12_plane_formats);
> Same as above, can this merge into condition just above this if ?
> 
> - Shashank
> >   		} else {
> >   			plane_formats = skl_plane_formats;
> >   			num_plane_formats =
> ARRAY_SIZE(skl_plane_formats);

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

Re: [PATCH] drm/i915: NV12 changes for Gen10

[Srinivas, Vidya]

Apologies. This patch was pushed wrongly. Not a part of the 16 patch series of NV12.

Regards
Vidya

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 6:17 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [PATCH] drm/i915: NV12 changes for Gen10
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 4:36 PM, Vidya Srinivas wrote:
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> No need for any commit message :P ?
> > ---
> >   drivers/gpu/drm/i915/intel_atomic.c  | 5 ++---
> >   drivers/gpu/drm/i915/intel_display.c | 7 ++++++-
> >   drivers/gpu/drm/i915/intel_sprite.c  | 4 ++++
> >   3 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > b/drivers/gpu/drm/i915/intel_atomic.c
> > index ecba7c7..ca213bc 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -327,14 +327,13 @@ int intel_atomic_setup_scalers(struct
> drm_i915_private *dev_priv,
> >   		}
> >
> >   		/* set scaler mode */
> > -		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv)) &&
> > +		if ((IS_BROXTON(dev_priv) || IS_KABYLAKE(dev_priv) ||
> > +			IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) &&
> Please align the conditions to above line, below existing IS_BXT
> >   			plane_state && plane_state->base.fb &&
> >   			plane_state->base.fb->format->format ==
> >   			DRM_FORMAT_NV12) {
> >   			scaler_state->scalers[*scaler_id].mode =
> >   				PS_SCALER_MODE_NV12;
> > -		} else if (IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) {
> > -			scaler_state->scalers[*scaler_id].mode = 0;
> >   		} else if (num_scalers_need == 1 && intel_crtc->pipe !=
> PIPE_C) {
> >   			/*
> >   			 * when only 1 scaler is in use on either pipe A or B,
> diff --git
> > a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index df6b11a..115e0ea 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13200,7 +13200,11 @@ intel_primary_plane_create(struct
> drm_i915_private *dev_priv, enum pipe pipe)
> >   			((pipe == PIPE_A || pipe == PIPE_B))) {
> >   			intel_primary_formats = nv12_primary_formats;
> >   			num_formats =
> ARRAY_SIZE(nv12_primary_formats);
> > +		} else if (IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) {
> > +			intel_primary_formats = nv12_primary_formats;
> > +			num_formats =
> ARRAY_SIZE(nv12_primary_formats);
> I can see these same lines in the above condition, can we simply add this (||
> IS_GLK || IS_CNL) condition in above ? Or not ?
> >   		} else {
> > +
> Extra line
> >   			intel_primary_formats = skl_primary_formats;
> >   			num_formats = ARRAY_SIZE(skl_primary_formats);
> >   		}
> > @@ -14006,7 +14010,8 @@ static int intel_framebuffer_init(struct
> intel_framebuffer *intel_fb,
> >   		}
> >   		break;
> >   	case DRM_FORMAT_NV12:
> > -		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv)) {
> > +		if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv) &&
> > +			!IS_GEMINILAKE(dev_priv) &&
> !IS_CANNONLAKE(dev_priv)) {
> Alignment
> >   			DRM_DEBUG_KMS("unsupported pixel format:
> %s\n",
> >   		      drm_get_format_name(mode_cmd->pixel_format,
> >   				&format_name));
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 1d35a18..35dcba0 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1340,6 +1340,10 @@ intel_sprite_plane_create(struct
> drm_i915_private *dev_priv,
> >   			(pipe == PIPE_A || pipe == PIPE_B) && plane == 0) {
> >   			plane_formats = nv12_plane_formats;
> >   			num_plane_formats =
> ARRAY_SIZE(nv12_plane_formats);
> > +		} else if ((IS_GEMINILAKE(dev_priv) ||
> IS_CANNONLAKE(dev_priv)) &&
> > +				   plane == 0) {
> > +			plane_formats = nv12_plane_formats;
> > +			num_plane_formats =
> ARRAY_SIZE(nv12_plane_formats);
> Same as above, can this merge into condition just above this if ?
> 
> - Shashank
> >   		} else {
> >   			plane_formats = skl_plane_formats;
> >   			num_plane_formats =
> ARRAY_SIZE(skl_plane_formats);

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

[PATCH 1/2] YUV444 10/12/16 bit declarations and additions

[Vidya Srinivas]

Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/drm_fourcc.c         |  3 +++
 drivers/gpu/drm/i915/i915_reg.h      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 42 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_sprite.c  |  7 +++++-
 include/uapi/drm/drm_fourcc.h        |  5 +++++
 5 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 9c0152d..cf70e79 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -173,6 +173,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .format = DRM_FORMAT_UYVY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
 		{ .format = DRM_FORMAT_VYUY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
 		{ .format = DRM_FORMAT_AYUV,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_Y410,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_Y412,		.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_Y416,		.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1 },
 	};
 
 	unsigned int i;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6db7d1a..5864030 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6400,6 +6400,9 @@ enum {
 #define _PLANE_NV12_BUF_CFG_1_A		0x70278
 #define _PLANE_NV12_BUF_CFG_2_A		0x70378
 
+#define	PLANE_CTL_FORMAT_Y410	(7 << 24)
+#define	PLANE_CTL_FORMAT_Y412	(9 << 24)
+#define	PLANE_CTL_FORMAT_Y416	(0xB << 24)
 
 #define _PLANE_CTL_1_B				0x71180
 #define _PLANE_CTL_2_B				0x71280
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index da9282d..546c79b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -87,6 +87,9 @@ static const uint32_t skl_primary_formats[] = {
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
 	DRM_FORMAT_NV12,
+	DRM_FORMAT_Y410,
+	DRM_FORMAT_Y412,
+	DRM_FORMAT_Y416,
 };
 
 static const uint64_t skl_format_modifiers_noccs[] = {
@@ -107,6 +110,21 @@ static const uint64_t skl_format_modifiers_ccs[] = {
 	DRM_FORMAT_MOD_INVALID
 };
 
+static const uint32_t gen11_primary_formats[] = {
+	DRM_FORMAT_C8,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+};
+
 /* Cursor formats */
 static const uint32_t intel_cursor_formats[] = {
 	DRM_FORMAT_ARGB8888,
@@ -3400,6 +3418,12 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
 		return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
 	case DRM_FORMAT_NV12:
 		return PLANE_CTL_FORMAT_NV12;
+	case DRM_FORMAT_Y410:
+		return PLANE_CTL_FORMAT_Y410;
+	case DRM_FORMAT_Y412:
+		return PLANE_CTL_FORMAT_Y412;
+	case DRM_FORMAT_Y416:
+		return PLANE_CTL_FORMAT_Y416;
 	default:
 		MISSING_CASE(pixel_format);
 	}
@@ -4780,6 +4804,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
 	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_Y410:
+	case DRM_FORMAT_Y412:
+	case DRM_FORMAT_Y416:
 		break;
 	default:
 		DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
@@ -13175,7 +13202,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		num_formats = ARRAY_SIZE(skl_primary_formats);
 
 		if (INTEL_GEN(dev_priv) == 9 && (pipe == PIPE_C))
-			num_formats -= 1;
+			num_formats -= 4;
+		else if (INTEL_GEN(dev_priv) == 9)
+			num_formats -= 3;
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
 			modifiers = skl_format_modifiers_ccs;
@@ -13371,6 +13400,8 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	struct intel_plane *cursor = NULL;
 	int sprite, ret;
 
+	DRM_ERROR("Vidya, Lets begin a new day\n");
+
 	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
 	if (!intel_crtc)
 		return -ENOMEM;
@@ -13986,6 +14017,15 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 			goto err;
 		}
 		break;
+	case DRM_FORMAT_Y410
+	case DRM_FORMAT_Y412:
+	case DRM_FORMAT_Y416:
+		if (INTEL_GEN(dev_priv) < 10) {
+			DRM_DEBUG_KMS("unsupported pixel format: %s\n",
+				      drm_get_format_name(mode_cmd->pixel_format, &format_name));
+			goto err;
+		}
+		break;
 	default:
 		DRM_DEBUG_KMS("unsupported pixel format: %s\n",
 			      drm_get_format_name(mode_cmd->pixel_format, &format_name));
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index f6f2ee8..2a2baa2 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1162,6 +1162,9 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_UYVY,
 	DRM_FORMAT_VYUY,
 	DRM_FORMAT_NV12,
+	DRM_FORMAT_Y410,
+	DRM_FORMAT_Y412,
+	DRM_FORMAT_Y416,
 };
 
 static const uint64_t skl_plane_format_modifiers_noccs[] = {
@@ -1362,7 +1365,9 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		if (INTEL_GEN(dev_priv) <= 10 && ((plane != 0) ||
 			(pipe == PIPE_C)))
-			num_plane_formats -= 1;
+			num_plane_formats -= 4;
+		else if (INTEL_GEN(dev_priv) <= 10)
+			num_plane_formats -= 3;
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
 			modifiers = skl_plane_format_modifiers_ccs;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index e04613d..c7922d3 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -113,6 +113,11 @@ extern "C" {
 
 #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
 
+#define DRM_FORMAT_Y410		fourcc_code('Y', '4', '1', '0') /* [31:0] A:V:Y:U 2:10:10:10 little endian */
+#define DRM_FORMAT_Y412		fourcc_code('Y', '4', '1', '2') /* [64:0] A:V:Y:U 12:12:12:12 little endian */
+#define DRM_FORMAT_Y416		fourcc_code('Y', '4', '1', '6') /* [64:0] A:V:Y:U 16:16:16:16 little endian */
+
+
 /*
  * 2 plane RGB + A
  * index 0 = RGB plane, same format as the corresponding non _A8 format has
-- 
2.7.4

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

[PATCH 2/2] yuv422 - rough patch for 10/12/16 bit packed support

[Vidya Srinivas]

Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/drm_fourcc.c         |  3 +++
 drivers/gpu/drm/i915/i915_reg.h      |  3 +++
 drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_sprite.c  | 12 +++++++++---
 include/uapi/drm/drm_fourcc.h        |  3 +++
 5 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index cf70e79..0468203 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -176,6 +176,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
 		{ .format = DRM_FORMAT_Y410,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_Y412,		.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1 },
 		{ .format = DRM_FORMAT_Y416,		.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1 },
+		{ .format = DRM_FORMAT_V210,		.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_V212,		.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1 },
+		{ .format = DRM_FORMAT_V216,		.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1 },
 	};
 
 	unsigned int i;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5864030..9ad7cd5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6403,6 +6403,9 @@ enum {
 #define	PLANE_CTL_FORMAT_Y410	(7 << 24)
 #define	PLANE_CTL_FORMAT_Y412	(9 << 24)
 #define	PLANE_CTL_FORMAT_Y416	(0xB << 24)
+#define	PLANE_CTL_FORMAT_V210	(1 << 24)
+#define	PLANE_CTL_FORMAT_V212	(3 << 24)
+#define	PLANE_CTL_FORMAT_V216	(5 << 24)
 
 #define _PLANE_CTL_1_B				0x71180
 #define _PLANE_CTL_2_B				0x71280
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 546c79b..567d8df 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -90,6 +90,9 @@ static const uint32_t skl_primary_formats[] = {
 	DRM_FORMAT_Y410,
 	DRM_FORMAT_Y412,
 	DRM_FORMAT_Y416,
+	DRM_FORMAT_V210,
+	DRM_FORMAT_V212,
+	DRM_FORMAT_V216,
 };
 
 static const uint64_t skl_format_modifiers_noccs[] = {
@@ -3424,6 +3427,12 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
 		return PLANE_CTL_FORMAT_Y412;
 	case DRM_FORMAT_Y416:
 		return PLANE_CTL_FORMAT_Y416;
+	case DRM_FORMAT_V210:
+		return PLANE_CTL_FORMAT_V210 | PLANE_CTL_YUV422_YUYV;
+	case DRM_FORMAT_V212:
+		return PLANE_CTL_FORMAT_V212 | PLANE_CTL_YUV422_YUYV;
+	case DRM_FORMAT_V216:
+		return PLANE_CTL_FORMAT_V216 | PLANE_CTL_YUV422_YUYV;
 	default:
 		MISSING_CASE(pixel_format);
 	}
@@ -4807,6 +4816,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_Y410:
 	case DRM_FORMAT_Y412:
 	case DRM_FORMAT_Y416:
+	case DRM_FORMAT_V210:
+	case DRM_FORMAT_V212:
+	case DRM_FORMAT_V216:
 		break;
 	default:
 		DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
@@ -12954,6 +12966,9 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_V210:
+	case DRM_FORMAT_V212:
+	case DRM_FORMAT_V216:
 		if (modifier == I915_FORMAT_MOD_Yf_TILED)
 			return true;
 		/* fall through */
@@ -13202,9 +13217,9 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 		num_formats = ARRAY_SIZE(skl_primary_formats);
 
 		if (INTEL_GEN(dev_priv) == 9 && (pipe == PIPE_C))
-			num_formats -= 4;
+			num_formats -= 7;
 		else if (INTEL_GEN(dev_priv) == 9)
-			num_formats -= 3;
+			num_formats -= 6;
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
 			modifiers = skl_format_modifiers_ccs;
@@ -14003,6 +14018,9 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_V210:
+	case DRM_FORMAT_V212:
+	case DRM_FORMAT_V216:
 		if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
 			DRM_DEBUG_KMS("unsupported pixel format: %s\n",
 				      drm_get_format_name(mode_cmd->pixel_format, &format_name));
@@ -14017,7 +14035,7 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 			goto err;
 		}
 		break;
-	case DRM_FORMAT_Y410
+	case DRM_FORMAT_Y410:
 	case DRM_FORMAT_Y412:
 	case DRM_FORMAT_Y416:
 		if (INTEL_GEN(dev_priv) < 10) {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 2a2baa2..a06576f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1165,6 +1165,9 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_Y410,
 	DRM_FORMAT_Y412,
 	DRM_FORMAT_Y416,
+	DRM_FORMAT_V210,
+	DRM_FORMAT_V212,
+	DRM_FORMAT_V216,
 };
 
 static const uint64_t skl_plane_format_modifiers_noccs[] = {
@@ -1193,7 +1196,7 @@ static bool g4x_mod_supported(uint32_t format, uint64_t modifier)
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
-		if (modifier == DRM_FORMAT_MOD_LINEAR ||
+	if (modifier == DRM_FORMAT_MOD_LINEAR ||
 		    modifier == I915_FORMAT_MOD_X_TILED)
 			return true;
 		/* fall through */
@@ -1261,6 +1264,9 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_V210:
+	case DRM_FORMAT_V212:
+	case DRM_FORMAT_V216:
 		if (modifier == I915_FORMAT_MOD_Yf_TILED)
 			return true;
 		/* fall through */
@@ -1365,9 +1371,9 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 
 		if (INTEL_GEN(dev_priv) <= 10 && ((plane != 0) ||
 			(pipe == PIPE_C)))
-			num_plane_formats -= 4;
+			num_plane_formats -= 7;
 		else if (INTEL_GEN(dev_priv) <= 10)
-			num_plane_formats -= 3;
+			num_plane_formats -= 6;
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
 			modifiers = skl_plane_format_modifiers_ccs;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index c7922d3..a82f74a 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -117,6 +117,9 @@ extern "C" {
 #define DRM_FORMAT_Y412		fourcc_code('Y', '4', '1', '2') /* [64:0] A:V:Y:U 12:12:12:12 little endian */
 #define DRM_FORMAT_Y416		fourcc_code('Y', '4', '1', '6') /* [64:0] A:V:Y:U 16:16:16:16 little endian */
 
+#define DRM_FORMAT_V210		fourcc_code('V', '2', '1', '0') /* [31:0] A:V:Y:U 2:10:10:10 little endian */
+#define DRM_FORMAT_V212		fourcc_code('V', '2', '1', '2') /* [64:0] A:V:Y:U 12:12:12:12 little endian */
+#define DRM_FORMAT_V216		fourcc_code('V', '2', '1', '6') /* [64:0] A:V:Y:U 16:16:16:16 little endian */
 
 /*
  * 2 plane RGB + A
-- 
2.7.4

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

✗ Fi.CI.BAT: failure for drm/i915: Display WA #0827 for NV12 to RGB switch (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Display WA #0827 for NV12 to RGB switch (rev4)
URL   : https://patchwork.freedesktop.org/series/37729/
State : failure

== Summary ==

Applying: YUV444 10/12/16 bit declarations and additions
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/drm_fourcc.c
M	drivers/gpu/drm/i915/i915_reg.h
M	drivers/gpu/drm/i915/intel_display.c
M	drivers/gpu/drm/i915/intel_sprite.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_sprite.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_sprite.c
Auto-merging drivers/gpu/drm/i915/intel_display.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_display.c
Auto-merging drivers/gpu/drm/i915/i915_reg.h
Auto-merging drivers/gpu/drm/drm_fourcc.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/drm_fourcc.c
Patch failed at 0001 YUV444 10/12/16 bit declarations and additions
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch

[David Weinehall]

On Tue, Feb 06, 2018 at 04:36:42PM +0530, Vidya Srinivas wrote:
> From: Chandra Konduru <chandra.konduru@intel.com>
> 
> Display WA #0827:
> Switching the plane format from NV12 to RGB and leaving system idle results
> in display underrun and corruption. WA: Set the bit 15 & bit 19 to 1b
> in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane is enabled.
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h      |  3 +++
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8f36023..c4af05e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3822,6 +3822,9 @@ enum {
>  #define _CLKGATE_DIS_PSL_A		0x46520
>  #define _CLKGATE_DIS_PSL_B		0x46524
>  #define _CLKGATE_DIS_PSL_C		0x46528
> +#define DUPS1_GATING_DIS	(1 << 15)
> +#define DUPS2_GATING_DIS	(1 << 19)
> +#define DUPS3_GATING_DIS	(1 << 23)
>  #define   DPF_GATING_DIS		(1 << 10)
>  #define   DPF_RAM_GATING_DIS		(1 << 9)
>  #define   DPFR_GATING_DIS		(1 << 8)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 551c970..94faf3e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5495,6 +5495,20 @@ static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
>  	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);
>  }
>  
> +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> +	int pipe, int enable)
> +{
> +	if (pipe == PIPE_A || pipe == PIPE_B) {
> +		if (enable)
> +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> +				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> +		else
> +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> +			I915_READ(CLKGATE_DIS_PSL(pipe)) &
> +			~(DUPS1_GATING_DIS|DUPS2_GATING_DIS));
> +	}
> +}
> +
>  static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  				struct drm_atomic_state *old_state)
>  {
> @@ -5599,6 +5613,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>  		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
>  		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
>  	}
> +	skl_wa_clkgate(dev_priv, pipe, 1);
>  }
>  
>  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
> @@ -5709,6 +5724,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
>  		intel_ddi_disable_pipe_clock(intel_crtc->config);
>  
>  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> +	skl_wa_clkgate(dev_priv, intel_crtc->pipe, 0);
>  }

Unless I'm misreading the context of this patch you're applying a workaround,
that by name seems to be for Skylake only, to: Haswell, Broadwell, and gen9+.

Either the name is incorrect, or the application of it.

As per BSpec the workaround seems to be for all of gen9 and only A-stepping of gen10.
I don't see it listed for Haswell or Broadwell.

Cross-referencing the WA-database with Bspec, based on the HSD
link, it seems that this issue *might* be
WaDups1GatingDisableClockGatingForMPO; if this is the case it might
make sense to include that WA name too. At the very least
there should always be a comment mentioning the workaround name/number
and the platform(s) it applies to.

Also, according to the WA database, if the above mentioned issue really
is the same, the WA is *NOT* necessary on GLK (seeing as GLK uses gen10
display this might make sense, though the WA database sometimes contains
mistakes).


Regards, David

>  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> -- 
> 1.9.1
> 
> _______________________________________________
> 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] drm/i915: Display WA #0827 for NV12 to RGB switch

[Srinivas, Vidya]

Sorry, my bad. This was a wrong push from my end. I have changed the tag to Not applicable.
Apologies.

Have sent out the NV12 series separately.

Regards
Vidya

> -----Original Message-----
> From: David Weinehall [mailto:david.weinehall@linux.intel.com]
> Sent: Tuesday, February 6, 2018 5:34 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Display WA #0827 for NV12 to
> RGB switch
> 
> On Tue, Feb 06, 2018 at 04:36:42PM +0530, Vidya Srinivas wrote:
> > From: Chandra Konduru <chandra.konduru@intel.com>
> >
> > Display WA #0827:
> > Switching the plane format from NV12 to RGB and leaving system idle
> > results in display underrun and corruption. WA: Set the bit 15 & bit
> > 19 to 1b in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane
> is enabled.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  3 +++
> >  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
> >  2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 8f36023..c4af05e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3822,6 +3822,9 @@ enum {
> >  #define _CLKGATE_DIS_PSL_A		0x46520
> >  #define _CLKGATE_DIS_PSL_B		0x46524
> >  #define _CLKGATE_DIS_PSL_C		0x46528
> > +#define DUPS1_GATING_DIS	(1 << 15)
> > +#define DUPS2_GATING_DIS	(1 << 19)
> > +#define DUPS3_GATING_DIS	(1 << 23)
> >  #define   DPF_GATING_DIS		(1 << 10)
> >  #define   DPF_RAM_GATING_DIS		(1 << 9)
> >  #define   DPFR_GATING_DIS		(1 << 8)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 551c970..94faf3e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5495,6 +5495,20 @@ static void
> glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
> >  	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);  }
> >
> > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > +	int pipe, int enable)
> > +{
> > +	if (pipe == PIPE_A || pipe == PIPE_B) {
> > +		if (enable)
> > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> > +		else
> > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +			I915_READ(CLKGATE_DIS_PSL(pipe)) &
> > +			~(DUPS1_GATING_DIS|DUPS2_GATING_DIS));
> > +	}
> > +}
> > +
> >  static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >  				struct drm_atomic_state *old_state)  { @@
> -5599,6 +5613,7 @@
> > static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >  		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
> >  		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
> >  	}
> > +	skl_wa_clkgate(dev_priv, pipe, 1);
> >  }
> >
> >  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool
> > force) @@ -5709,6 +5724,7 @@ static void haswell_crtc_disable(struct
> intel_crtc_state *old_crtc_state,
> >  		intel_ddi_disable_pipe_clock(intel_crtc->config);
> >
> >  	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> > +	skl_wa_clkgate(dev_priv, intel_crtc->pipe, 0);
> >  }
> 
> Unless I'm misreading the context of this patch you're applying a
> workaround, that by name seems to be for Skylake only, to: Haswell,
> Broadwell, and gen9+.
> 
> Either the name is incorrect, or the application of it.
> 
> As per BSpec the workaround seems to be for all of gen9 and only A-
> stepping of gen10.
> I don't see it listed for Haswell or Broadwell.
> 
> Cross-referencing the WA-database with Bspec, based on the HSD link, it
> seems that this issue *might* be
> WaDups1GatingDisableClockGatingForMPO; if this is the case it might make
> sense to include that WA name too. At the very least there should always be
> a comment mentioning the workaround name/number and the platform(s)
> it applies to.
> 
> Also, according to the WA database, if the above mentioned issue really is
> the same, the WA is *NOT* necessary on GLK (seeing as GLK uses gen10
> display this might make sense, though the WA database sometimes
> contains mistakes).
> 
> 
> Regards, David
> 
> >  static void i9xx_pfit_enable(struct intel_crtc *crtc)
> > --
> > 1.9.1
> >
> > _______________________________________________
> > 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] drm/i915: Display WA #0827 for NV12 to RGB switch

[Maarten Lankhorst]

Hey,

Op 06-02-18 om 12:06 schreef Vidya Srinivas:
> From: Chandra Konduru <chandra.konduru@intel.com>
>
> Display WA #0827:
> Switching the plane format from NV12 to RGB and leaving system idle results
> in display underrun and corruption. WA: Set the bit 15 & bit 19 to 1b
> in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane is enabled.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>

Is it required to leave the workaround enabled all the time? And how can I reproduce it? I tried to write a
dumb testcase (below) to expose this issue, but didn't have much luck..
---
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 870c9093550b..f536db0fa433 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -192,6 +192,7 @@ TESTS_progs = \
 	kms_legacy_colorkey \
 	kms_mmap_write_crc \
 	kms_mmio_vs_cs_flip \
+	kms_nv12 \
 	kms_panel_fitting \
 	kms_pipe_b_c_ivb \
 	kms_pipe_crc_basic \
diff --git a/tests/kms_nv12.c b/tests/kms_nv12.c
new file mode 100644
index 000000000000..384a15afde52
--- /dev/null
+++ b/tests/kms_nv12.c
@@ -0,0 +1,195 @@
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Maarten Lankhorst 
+ */
+#include "config.h"
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+typedef struct {
+	igt_display_t display;
+
+	igt_pipe_crc_t *pipe_crc;
+	struct igt_fb fb[4];
+} data_t;
+
+static bool plane_supports_format(igt_plane_t *plane, uint32_t format)
+{
+	int i;
+
+	if (!igt_fb_supported_format(format))
+		return false;
+
+	for (i = 0; i < plane->drm_plane->count_formats; i++)
+		if (plane->drm_plane->formats[i] == format)
+			return true;
+
+	return false;
+}
+
+static bool pipe_supports_format(igt_display_t *display, enum pipe pipe, uint32_t format)
+{
+	igt_plane_t *plane;
+
+	for_each_plane_on_pipe(display, pipe, plane)
+		if (plane_supports_format(plane, format))
+			return true;
+
+	return false;
+}
+
+static void remove_fbs(data_t *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(data->fb); i++)
+		igt_remove_fb(data->display.drm_fd, &data->fb[i]);
+}
+
+static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+	igt_display_t *display = &data->display;
+
+	remove_fbs(data);
+	igt_display_reset(display);
+	igt_output_set_pipe(output, pipe);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	igt_pipe_crc_free(data->pipe_crc);
+	data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+}
+
+static void set_fb(igt_plane_t *plane, struct igt_fb *fb)
+{
+	igt_plane_set_fb(plane, fb);
+
+	if (fb && fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED) {
+		igt_plane_set_rotation(plane, IGT_ROTATION_90);
+		igt_plane_set_size(plane, fb->height, fb->width);
+	} else
+		igt_plane_set_rotation(plane, IGT_ROTATION_0);
+}
+
+static void nv12_rgb_switch(data_t *data, enum pipe pipe, igt_output_t *output)
+{
+	drmModeModeInfo *mode = igt_output_get_mode(output);
+	igt_display_t *display = &data->display;
+	igt_plane_t *plane;
+	int i;
+	igt_crc_t ref_crc[4], crc;
+
+	prepare_crtc(data, pipe, output);
+
+	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      DRM_FORMAT_NV12, LOCAL_I915_FORMAT_MOD_X_TILED,
+			      &data->fb[0]);
+
+  	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
+			      DRM_FORMAT_NV12, LOCAL_I915_FORMAT_MOD_Y_TILED,
+			      &data->fb[1]);
+
+	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
+			      DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_X_TILED,
+			      &data->fb[2]);
+
+	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
+			      DRM_FORMAT_XRGB8888, LOCAL_I915_FORMAT_MOD_Y_TILED,
+			      &data->fb[3]);
+
+	for_each_plane_on_pipe(display, pipe, plane)
+		if (plane->type != DRM_PLANE_TYPE_CURSOR)
+			set_fb(plane, &data->fb[3]);
+
+	for_each_plane_on_pipe(display, pipe, plane) {
+		const int seq[] = {
+			2, 0, 2, 1, 3, 1, 3
+		};
+
+		if (!plane_supports_format(plane, DRM_FORMAT_NV12))
+			continue;
+
+		/* Collect reference crc with toggle in between. */
+		for (i = 0; i < ARRAY_SIZE(ref_crc); i++) {
+			set_fb(plane, &data->fb[i]);
+			igt_display_commit2(display, COMMIT_ATOMIC);
+
+			igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc[i]);
+
+			set_fb(plane, NULL);
+			igt_display_commit2(display, COMMIT_ATOMIC);
+		}
+
+		for (i = 0; i < ARRAY_SIZE(seq); i++) {
+			int j = seq[i];
+
+			set_fb(plane, &data->fb[j]);
+			igt_display_commit2(display, COMMIT_ATOMIC);
+
+			igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+			igt_assert_crc_equal(&ref_crc[j], &crc);
+		}
+	}
+}
+
+static void run_tests_for_pipe(data_t *data, enum pipe pipe)
+{
+	igt_output_t *output;
+	igt_display_t *display = &data->display;
+
+	igt_fixture {
+		igt_display_require_output_on_pipe(display, pipe);
+		igt_require(pipe_supports_format(display, pipe, DRM_FORMAT_NV12));
+	}
+
+	igt_subtest_f("pipe-%s-nv12-rgb-switch", kmstest_pipe_name(pipe))
+		for_each_valid_output_on_pipe(display, pipe, output)
+			nv12_rgb_switch(data, pipe, output);
+}
+
+igt_main
+{
+	data_t data = {};
+	enum pipe pipe;
+
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		data.display.drm_fd = drm_open_driver_master(DRIVER_ANY);
+
+		kmstest_set_vt_graphics_mode();
+		igt_display_init(&data.display, data.display.drm_fd);
+		igt_require(data.display.is_atomic);
+	}
+
+	for_each_pipe_static(pipe)
+		igt_subtest_group
+			run_tests_for_pipe(&data, pipe);
+
+	igt_fixture {
+		igt_display_fini(&data.display);
+		close(data.display.drm_fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 521a4c425a68..cfcd476ccd10 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -170,6 +170,7 @@ test_progs = [
 	'kms_legacy_colorkey',
 	'kms_mmap_write_crc',
 	'kms_mmio_vs_cs_flip',
+	'kms_nv12',
 	'kms_panel_fitting',
 	'kms_pipe_b_c_ivb',
 	'kms_pipe_crc_basic',

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

Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch

[Srinivas, Vidya]

Hi Maarten

Sorry, my bad. This was a wrong push from my end. I have changed the tag to Not applicable.
Apologies.

Have sent out the NV12 series separately.

Regards
Vidya

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Tuesday, February 6, 2018 7:33 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Kamath, Sunil <sunil.kamath@intel.com>; Sharma, Shashank
> <shashank.sharma@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Konduru, Chandra <chandra.konduru@intel.com>
> Subject: Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch
> 
> Hey,
> 
> Op 06-02-18 om 12:06 schreef Vidya Srinivas:
> > From: Chandra Konduru <chandra.konduru@intel.com>
> >
> > Display WA #0827:
> > Switching the plane format from NV12 to RGB and leaving system idle
> > results in display underrun and corruption. WA: Set the bit 15 & bit
> > 19 to 1b in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane
> is enabled.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> 
> Is it required to leave the workaround enabled all the time? And how can I
> reproduce it? I tried to write a dumb testcase (below) to expose this issue,
> but didn't have much luck..
> ---
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
> 870c9093550b..f536db0fa433 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -192,6 +192,7 @@ TESTS_progs = \
>  	kms_legacy_colorkey \
>  	kms_mmap_write_crc \
>  	kms_mmio_vs_cs_flip \
> +	kms_nv12 \
>  	kms_panel_fitting \
>  	kms_pipe_b_c_ivb \
>  	kms_pipe_crc_basic \
> diff --git a/tests/kms_nv12.c b/tests/kms_nv12.c new file mode 100644
> index 000000000000..384a15afde52
> --- /dev/null
> +++ b/tests/kms_nv12.c
> @@ -0,0 +1,195 @@
> +/*
> + * Permission is hereby granted, free of charge, to any person
> +obtaining a
> + * copy of this software and associated documentation files (the
> +"Software"),
> + * to deal in the Software without restriction, including without
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> EVENT
> +SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES OR
> +OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> OTHERWISE,
> +ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> OR OTHER
> +DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Maarten Lankhorst
> + */
> +#include "config.h"
> +
> +#include "igt.h"
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +typedef struct {
> +	igt_display_t display;
> +
> +	igt_pipe_crc_t *pipe_crc;
> +	struct igt_fb fb[4];
> +} data_t;
> +
> +static bool plane_supports_format(igt_plane_t *plane, uint32_t format)
> +{
> +	int i;
> +
> +	if (!igt_fb_supported_format(format))
> +		return false;
> +
> +	for (i = 0; i < plane->drm_plane->count_formats; i++)
> +		if (plane->drm_plane->formats[i] == format)
> +			return true;
> +
> +	return false;
> +}
> +
> +static bool pipe_supports_format(igt_display_t *display, enum pipe
> +pipe, uint32_t format) {
> +	igt_plane_t *plane;
> +
> +	for_each_plane_on_pipe(display, pipe, plane)
> +		if (plane_supports_format(plane, format))
> +			return true;
> +
> +	return false;
> +}
> +
> +static void remove_fbs(data_t *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->fb); i++)
> +		igt_remove_fb(data->display.drm_fd, &data->fb[i]); }
> +
> +static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t
> +*output) {
> +	igt_display_t *display = &data->display;
> +
> +	remove_fbs(data);
> +	igt_display_reset(display);
> +	igt_output_set_pipe(output, pipe);
> +	igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +	igt_pipe_crc_free(data->pipe_crc);
> +	data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe,
> +INTEL_PIPE_CRC_SOURCE_AUTO); }
> +
> +static void set_fb(igt_plane_t *plane, struct igt_fb *fb) {
> +	igt_plane_set_fb(plane, fb);
> +
> +	if (fb && fb->tiling == LOCAL_I915_FORMAT_MOD_Y_TILED) {
> +		igt_plane_set_rotation(plane, IGT_ROTATION_90);
> +		igt_plane_set_size(plane, fb->height, fb->width);
> +	} else
> +		igt_plane_set_rotation(plane, IGT_ROTATION_0); }
> +
> +static void nv12_rgb_switch(data_t *data, enum pipe pipe, igt_output_t
> +*output) {
> +	drmModeModeInfo *mode = igt_output_get_mode(output);
> +	igt_display_t *display = &data->display;
> +	igt_plane_t *plane;
> +	int i;
> +	igt_crc_t ref_crc[4], crc;
> +
> +	prepare_crtc(data, pipe, output);
> +
> +	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> +			      DRM_FORMAT_NV12,
> LOCAL_I915_FORMAT_MOD_X_TILED,
> +			      &data->fb[0]);
> +
> +  	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode-
> >hdisplay,
> +			      DRM_FORMAT_NV12,
> LOCAL_I915_FORMAT_MOD_Y_TILED,
> +			      &data->fb[1]);
> +
> +	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode-
> >vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> LOCAL_I915_FORMAT_MOD_X_TILED,
> +			      &data->fb[2]);
> +
> +	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode-
> >hdisplay,
> +			      DRM_FORMAT_XRGB8888,
> LOCAL_I915_FORMAT_MOD_Y_TILED,
> +			      &data->fb[3]);
> +
> +	for_each_plane_on_pipe(display, pipe, plane)
> +		if (plane->type != DRM_PLANE_TYPE_CURSOR)
> +			set_fb(plane, &data->fb[3]);
> +
> +	for_each_plane_on_pipe(display, pipe, plane) {
> +		const int seq[] = {
> +			2, 0, 2, 1, 3, 1, 3
> +		};
> +
> +		if (!plane_supports_format(plane, DRM_FORMAT_NV12))
> +			continue;
> +
> +		/* Collect reference crc with toggle in between. */
> +		for (i = 0; i < ARRAY_SIZE(ref_crc); i++) {
> +			set_fb(plane, &data->fb[i]);
> +			igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +			igt_pipe_crc_collect_crc(data->pipe_crc,
> &ref_crc[i]);
> +
> +			set_fb(plane, NULL);
> +			igt_display_commit2(display, COMMIT_ATOMIC);
> +		}
> +
> +		for (i = 0; i < ARRAY_SIZE(seq); i++) {
> +			int j = seq[i];
> +
> +			set_fb(plane, &data->fb[j]);
> +			igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +			igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> +			igt_assert_crc_equal(&ref_crc[j], &crc);
> +		}
> +	}
> +}
> +
> +static void run_tests_for_pipe(data_t *data, enum pipe pipe) {
> +	igt_output_t *output;
> +	igt_display_t *display = &data->display;
> +
> +	igt_fixture {
> +		igt_display_require_output_on_pipe(display, pipe);
> +		igt_require(pipe_supports_format(display, pipe,
> DRM_FORMAT_NV12));
> +	}
> +
> +	igt_subtest_f("pipe-%s-nv12-rgb-switch",
> kmstest_pipe_name(pipe))
> +		for_each_valid_output_on_pipe(display, pipe, output)
> +			nv12_rgb_switch(data, pipe, output); }
> +
> +igt_main
> +{
> +	data_t data = {};
> +	enum pipe pipe;
> +
> +	igt_skip_on_simulation();
> +
> +	igt_fixture {
> +		data.display.drm_fd =
> drm_open_driver_master(DRIVER_ANY);
> +
> +		kmstest_set_vt_graphics_mode();
> +		igt_display_init(&data.display, data.display.drm_fd);
> +		igt_require(data.display.is_atomic);
> +	}
> +
> +	for_each_pipe_static(pipe)
> +		igt_subtest_group
> +			run_tests_for_pipe(&data, pipe);
> +
> +	igt_fixture {
> +		igt_display_fini(&data.display);
> +		close(data.display.drm_fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build index
> 521a4c425a68..cfcd476ccd10 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -170,6 +170,7 @@ test_progs = [
>  	'kms_legacy_colorkey',
>  	'kms_mmap_write_crc',
>  	'kms_mmio_vs_cs_flip',
> +	'kms_nv12',
>  	'kms_panel_fitting',
>  	'kms_pipe_b_c_ivb',
>  	'kms_pipe_crc_basic',

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

Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch

[Sharma, Shashank]

Regards

Shashank


On 2/6/2018 4:36 PM, Vidya Srinivas wrote:
> From: Chandra Konduru <chandra.konduru@intel.com>
>
> Display WA #0827:
> Switching the plane format from NV12 to RGB and leaving system idle results
> in display underrun and corruption. WA: Set the bit 15 & bit 19 to 1b
> in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane is enabled.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h      |  3 +++
>   drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
>   2 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8f36023..c4af05e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3822,6 +3822,9 @@ enum {
>   #define _CLKGATE_DIS_PSL_A		0x46520
>   #define _CLKGATE_DIS_PSL_B		0x46524
>   #define _CLKGATE_DIS_PSL_C		0x46528
> +#define DUPS1_GATING_DIS	(1 << 15)
> +#define DUPS2_GATING_DIS	(1 << 19)
> +#define DUPS3_GATING_DIS	(1 << 23)
Bit definition should be aligned by one extra space (like below), also 
the bit sequence should be high -> low (so 23,19 and then 15)
>   #define   DPF_GATING_DIS		(1 << 10)
>   #define   DPF_RAM_GATING_DIS		(1 << 9)
>   #define   DPFR_GATING_DIS		(1 << 8)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 551c970..94faf3e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5495,6 +5495,20 @@ static void glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
>   	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);
>   }
>   
> +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> +	int pipe, int enable)
Do we need an int ? or bool enable ? also This line should be aligned to 
opening brace '(' above.
> +{
> +	if (pipe == PIPE_A || pipe == PIPE_B) {
> +		if (enable)
> +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> +				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
Alignment, also we are overwriting all other bits here, this should be 
I915_WRITE(CLKGATE_DIS_PSL(pipe), I915_READ(CLKGATE_DIS_PSL(pipe)) |= 
(DUPS1_GATING_DIS | DUPS2_GATING_DIS) )
> +		else
> +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> +			I915_READ(CLKGATE_DIS_PSL(pipe)) &
This line should be aligned to the '(' above
> +			~(DUPS1_GATING_DIS|DUPS2_GATING_DIS));
> +	}
> +}
> +
>   static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>   				struct drm_atomic_state *old_state)
>   {
> @@ -5599,6 +5613,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
>   		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
>   		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
>   	}
> +	skl_wa_clkgate(dev_priv, pipe, 1);
send true from here (instead of 1)
>   }
>   
>   static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force)
> @@ -5709,6 +5724,7 @@ static void haswell_crtc_disable(struct intel_crtc_state *old_crtc_state,
>   		intel_ddi_disable_pipe_clock(intel_crtc->config);
>   
>   	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> +	skl_wa_clkgate(dev_priv, intel_crtc->pipe, 0);
send false from here (instead of 0)
>   }
>   
>   static void i9xx_pfit_enable(struct intel_crtc *crtc)

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

Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch

[Srinivas, Vidya]

Apologies. This patch was pushed wrongly. Not a part of the 16 patch series of NV12.

Regards
Vidya

> -----Original Message-----
> From: Sharma, Shashank
> Sent: Thursday, February 8, 2018 6:08 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: maarten.lankhorst@linux.intel.com; Kamath, Sunil
> <sunil.kamath@intel.com>; Shankar, Uma <uma.shankar@intel.com>;
> Konduru, Chandra <chandra.konduru@intel.com>
> Subject: Re: [PATCH] drm/i915: Display WA #0827 for NV12 to RGB switch
> 
> Regards
> 
> Shashank
> 
> 
> On 2/6/2018 4:36 PM, Vidya Srinivas wrote:
> > From: Chandra Konduru <chandra.konduru@intel.com>
> >
> > Display WA #0827:
> > Switching the plane format from NV12 to RGB and leaving system idle
> > results in display underrun and corruption. WA: Set the bit 15 & bit
> > 19 to 1b in the CLKGATE_DIS_PSL register for the pipe in which NV12 plane
> is enabled.
> >
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h      |  3 +++
> >   drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++++
> >   2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 8f36023..c4af05e 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3822,6 +3822,9 @@ enum {
> >   #define _CLKGATE_DIS_PSL_A		0x46520
> >   #define _CLKGATE_DIS_PSL_B		0x46524
> >   #define _CLKGATE_DIS_PSL_C		0x46528
> > +#define DUPS1_GATING_DIS	(1 << 15)
> > +#define DUPS2_GATING_DIS	(1 << 19)
> > +#define DUPS3_GATING_DIS	(1 << 23)
> Bit definition should be aligned by one extra space (like below), also the bit
> sequence should be high -> low (so 23,19 and then 15)
> >   #define   DPF_GATING_DIS		(1 << 10)
> >   #define   DPF_RAM_GATING_DIS		(1 << 9)
> >   #define   DPFR_GATING_DIS		(1 << 8)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 551c970..94faf3e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5495,6 +5495,20 @@ static void
> glk_pipe_scaler_clock_gating_wa(struct drm_i915_private *dev_priv,
> >   	I915_WRITE(CLKGATE_DIS_PSL(pipe), val);
> >   }
> >
> > +static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
> > +	int pipe, int enable)
> Do we need an int ? or bool enable ? also This line should be aligned to
> opening brace '(' above.
> > +{
> > +	if (pipe == PIPE_A || pipe == PIPE_B) {
> > +		if (enable)
> > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
> Alignment, also we are overwriting all other bits here, this should be
> I915_WRITE(CLKGATE_DIS_PSL(pipe), I915_READ(CLKGATE_DIS_PSL(pipe))
> |= (DUPS1_GATING_DIS | DUPS2_GATING_DIS) )
> > +		else
> > +			I915_WRITE(CLKGATE_DIS_PSL(pipe),
> > +			I915_READ(CLKGATE_DIS_PSL(pipe)) &
> This line should be aligned to the '(' above
> > +			~(DUPS1_GATING_DIS|DUPS2_GATING_DIS));
> > +	}
> > +}
> > +
> >   static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> >   				struct drm_atomic_state *old_state)
> >   {
> > @@ -5599,6 +5613,7 @@ static void haswell_crtc_enable(struct
> intel_crtc_state *pipe_config,
> >   		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
> >   		intel_wait_for_vblank(dev_priv, hsw_workaround_pipe);
> >   	}
> > +	skl_wa_clkgate(dev_priv, pipe, 1);
> send true from here (instead of 1)
> >   }
> >
> >   static void ironlake_pfit_disable(struct intel_crtc *crtc, bool
> > force) @@ -5709,6 +5724,7 @@ static void haswell_crtc_disable(struct
> intel_crtc_state *old_crtc_state,
> >   		intel_ddi_disable_pipe_clock(intel_crtc->config);
> >
> >   	intel_encoders_post_disable(crtc, old_crtc_state, old_state);
> > +	skl_wa_clkgate(dev_priv, intel_crtc->pipe, 0);
> send false from here (instead of 0)
> >   }
> >
> >   static void i9xx_pfit_enable(struct intel_crtc *crtc)

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