[PATCH v6 0/6] Enable NV12 support

[PATCH v6 0/6] Enable NV12 support

[Vidya Srinivas]

Enabling NV12 support:
- Framebuffer creation
- Primary and Sprite plane support
Patch series depend on Enable display workaround 827 patch
mentioned below submitted by Maarten
Removed BXT support for NV12 due to WA826

Changes from prev version:
Rebased the series, changed the order as suggested by Maarten

Chandra Konduru (3):
  drm/i915: Add NV12 support to intel_framebuffer_init
  drm/i915: Add NV12 as supported format for primary plane
  drm/i915: Add NV12 as supported format for sprite plane

Maarten Lankhorst (2):
  drm/i915: Enable display workaround 827 for all planes, v2.
  drm/i915: Add skl_check_nv12_surface for NV12

Srinivas, Vidya (1):
  drm/i915: Enable Display WA 0528

 drivers/gpu/drm/i915/intel_atomic_plane.c |   7 +-
 drivers/gpu/drm/i915/intel_display.c      | 178 ++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h          |   3 +
 drivers/gpu/drm/i915/intel_sprite.c       |  30 ++++-
 4 files changed, 194 insertions(+), 24 deletions(-)

-- 
2.7.4

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

[PATCH v6 2/6] drm/i915: Enable Display WA 0528

[Vidya Srinivas]

From: "Srinivas, Vidya" <vidya.srinivas@intel.com>

Possible hang with NV12 plane surface formats.
WA: When the plane source pixel format is NV12,
the CHICKEN_PIPESL_* register bit 22 must be set to 1
and the render decompression must not be enabled
on any of the planes in that pipe.

v2: removed unnecessary POSTING_READ

v3: Added RB from Maarten

v4: Removed support for NV12 for BROXTON

Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 871347f..4279188 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -489,9 +489,21 @@ static const struct intel_limit intel_limits_bxt = {
 };
 
 static void
+skl_wa_528(struct drm_i915_private *dev_priv, int pipe, bool enable)
+{
+	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
+		return;
+
+	if (enable)
+		I915_WRITE(CHICKEN_PIPESL_1(pipe), HSW_FBCQ_DIS);
+	else
+		I915_WRITE(CHICKEN_PIPESL_1(pipe), 0);
+}
+
+static void
 skl_wa_clkgate(struct drm_i915_private *dev_priv, int pipe, bool enable)
 {
-	if (IS_SKYLAKE(dev_priv))
+	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
 		return;
 
 	if (enable)
@@ -5193,8 +5205,10 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 
 	/* Display WA 827 */
 	if (needs_nv12_wa(dev_priv, old_crtc_state) &&
-	    !needs_nv12_wa(dev_priv, pipe_config))
+	    !needs_nv12_wa(dev_priv, pipe_config)) {
 		skl_wa_clkgate(dev_priv, crtc->pipe, false);
+		skl_wa_528(dev_priv, crtc->pipe, false);
+	}
 }
 
 static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
@@ -5231,8 +5245,10 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 
 	/* Display WA 827 */
 	if (!needs_nv12_wa(dev_priv, old_crtc_state) &&
-	    needs_nv12_wa(dev_priv, pipe_config))
+	    needs_nv12_wa(dev_priv, pipe_config)) {
 		skl_wa_clkgate(dev_priv, crtc->pipe, true);
+		skl_wa_528(dev_priv, crtc->pipe, true);
+	}
 
 	/*
 	 * Vblank time updates from the shadow to live plane control register
-- 
2.7.4

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

[PATCH v6 1/6] drm/i915: Enable display workaround 827 for all planes, v2.

[Vidya Srinivas]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

The workaround was applied only to the primary plane, but is required
on all planes. Iterate over all planes in the crtc atomic check to see
if the workaround is enabled, and only perform the actual toggling in
the pre/post plane update functions.

Changes since v1:
- Track active NV12 planes in a nv12_planes bitmask. (Ville)

v2: Removing BROXTON support for NV12 due to WA826

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  7 ++++-
 drivers/gpu/drm/i915/intel_display.c      | 43 +++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_drv.h          |  1 +
 3 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 7481ce8..6d06878 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -183,11 +183,16 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 	}
 
 	/* FIXME pre-g4x don't work like this */
-	if (intel_state->base.visible)
+	if (state->visible)
 		crtc_state->active_planes |= BIT(intel_plane->id);
 	else
 		crtc_state->active_planes &= ~BIT(intel_plane->id);
 
+	if (state->visible && state->fb->format->format == DRM_FORMAT_NV12)
+		crtc_state->nv12_planes |= BIT(intel_plane->id);
+	else
+		crtc_state->nv12_planes &= ~BIT(intel_plane->id);
+
 	return intel_plane_atomic_calc_changes(old_crtc_state,
 					       &crtc_state->base,
 					       old_plane_state,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3fd249c..871347f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5142,6 +5142,22 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s
 	return !old_crtc_state->ips_enabled;
 }
 
+static bool needs_nv12_wa(struct drm_i915_private *dev_priv,
+			  const struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->nv12_planes)
+		return false;
+
+	if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
+		return false;
+
+	if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
+	       IS_CANNONLAKE(dev_priv))
+		return true;
+
+	return false;
+}
+
 static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
@@ -5166,7 +5182,6 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 	if (old_primary_state) {
 		struct drm_plane_state *new_primary_state =
 			drm_atomic_get_new_plane_state(old_state, primary);
-		struct drm_framebuffer *fb = new_primary_state->fb;
 
 		intel_fbc_post_update(crtc);
 
@@ -5174,15 +5189,12 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 		    (needs_modeset(&pipe_config->base) ||
 		     !old_primary_state->visible))
 			intel_post_enable_primary(&crtc->base, pipe_config);
-
-		/* Display WA 827 */
-		if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
-		    IS_CANNONLAKE(dev_priv)) {
-			if (fb && fb->format->format == DRM_FORMAT_NV12)
-				skl_wa_clkgate(dev_priv, crtc->pipe, false);
-		}
-
 	}
+
+	/* Display WA 827 */
+	if (needs_nv12_wa(dev_priv, old_crtc_state) &&
+	    !needs_nv12_wa(dev_priv, pipe_config))
+		skl_wa_clkgate(dev_priv, crtc->pipe, false);
 }
 
 static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
@@ -5206,14 +5218,6 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 		struct intel_plane_state *new_primary_state =
 			intel_atomic_get_new_plane_state(old_intel_state,
 							 to_intel_plane(primary));
-		struct drm_framebuffer *fb = new_primary_state->base.fb;
-
-		/* Display WA 827 */
-		if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
-		    IS_CANNONLAKE(dev_priv)) {
-			if (fb && fb->format->format == DRM_FORMAT_NV12)
-				skl_wa_clkgate(dev_priv, crtc->pipe, true);
-		}
 
 		intel_fbc_pre_update(crtc, pipe_config, new_primary_state);
 		/*
@@ -5225,6 +5229,11 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 			intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
 	}
 
+	/* Display WA 827 */
+	if (!needs_nv12_wa(dev_priv, old_crtc_state) &&
+	    needs_nv12_wa(dev_priv, pipe_config))
+		skl_wa_clkgate(dev_priv, crtc->pipe, true);
+
 	/*
 	 * Vblank time updates from the shadow to live plane control register
 	 * are blocked if the memory self-refresh mode is active at that
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 11a1932..9a13eeb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -890,6 +890,7 @@ struct intel_crtc_state {
 
 	/* bitmask of visible planes (enum plane_id) */
 	u8 active_planes;
+	u8 nv12_planes;
 
 	/* HDMI scrambling status */
 	bool hdmi_scrambling;
-- 
2.7.4

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

[PATCH v6 3/6] drm/i915: Add skl_check_nv12_surface for NV12

[Vidya Srinivas]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

We skip src trunction/adjustments for
NV12 case and handle the sizes directly.
Without this, pipe fifo underruns are seen on APL/KBL.

v2: For NV12, making the src coordinates multiplier of 4

v3: Moving all the src coords handling code for NV12
to skl_check_nv12_surface

v4: Added RB from Mika

v5: Rebased the series. Removed checks of mult of 4 in
skl_update_scaler, Added NV12 condition in intel_check_sprite_plane
where src x/w is being checked for mult of 2 for yuv planes.

Reviewed-by: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_sprite.c  |  1 +
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4279188..f1263e9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3102,6 +3102,42 @@ static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
 	return 0;
 }
 
+static int
+skl_check_nv12_surface(const struct intel_crtc_state *crtc_state,
+		       struct intel_plane_state *plane_state)
+{
+	int crtc_x2 = plane_state->base.crtc_x + plane_state->base.crtc_w;
+	int crtc_y2 = plane_state->base.crtc_y + plane_state->base.crtc_h;
+
+	if (((plane_state->base.src_x >> 16) % 4) != 0 ||
+	    ((plane_state->base.src_y >> 16) % 4) != 0 ||
+	    ((plane_state->base.src_w >> 16) % 4) != 0 ||
+	    ((plane_state->base.src_h >> 16) % 4) != 0) {
+		DRM_DEBUG_KMS("src coords must be multiple of 4 for NV12\n");
+		return -EINVAL;
+	}
+
+	/* Clipping would cause a 1-3 pixel gap at the edge of the screen? */
+	if ((crtc_x2 > crtc_state->pipe_src_w && crtc_state->pipe_src_w % 4) ||
+	    (crtc_y2 > crtc_state->pipe_src_h && crtc_state->pipe_src_h % 4)) {
+		DRM_DEBUG_KMS("It's not possible to clip %u,%u to %u,%u\n",
+			      crtc_x2, crtc_y2,
+			      crtc_state->pipe_src_w, crtc_state->pipe_src_h);
+		return -EINVAL;
+	}
+
+	plane_state->base.src.x1 =
+		DIV_ROUND_CLOSEST(plane_state->base.src.x1, 1 << 18) << 18;
+	plane_state->base.src.x2 =
+		DIV_ROUND_CLOSEST(plane_state->base.src.x2, 1 << 18) << 18;
+	plane_state->base.src.y1 =
+		DIV_ROUND_CLOSEST(plane_state->base.src.y1, 1 << 18) << 18;
+	plane_state->base.src.y2 =
+		DIV_ROUND_CLOSEST(plane_state->base.src.y2, 1 << 18) << 18;
+
+	return 0;
+}
+
 static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
 {
 	const struct drm_framebuffer *fb = plane_state->base.fb;
@@ -3185,6 +3221,9 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
 	 * the main surface setup depends on it.
 	 */
 	if (fb->format->format == DRM_FORMAT_NV12) {
+		ret = skl_check_nv12_surface(crtc_state, plane_state);
+		if (ret)
+			return ret;
 		ret = skl_check_nv12_aux_surface(plane_state);
 		if (ret)
 			return ret;
@@ -4806,8 +4845,7 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 	}
 
 	if (plane_scaler_check && pixel_format == DRM_FORMAT_NV12 &&
-	    (src_h < SKL_MIN_YUV_420_SRC_H || (src_w % 4) != 0 ||
-	     (src_h % 4) != 0)) {
+	    (src_h < SKL_MIN_YUV_420_SRC_H || src_w < SKL_MIN_YUV_420_SRC_W)) {
 		DRM_DEBUG_KMS("NV12: src dimensions not met\n");
 		return -EINVAL;
 	}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0c394a2..c73553a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1011,6 +1011,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
 		src->y2 = (src_y + src_h) << 16;
 
 		if (intel_format_is_yuv(fb->format->format) &&
+		    fb->format->format != DRM_FORMAT_NV12 &&
 		    (src_x % 2 || src_w % 2)) {
 			DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 for YUV planes\n",
 				      src_x, src_w);
-- 
2.7.4

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

[PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Vidya Srinivas]

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

This patch adds NV12 to list of supported formats for sprite plane.

v2: Rebased (me)

v3: Review comments by Ville addressed
- Removed skl_plane_formats_with_nv12 and added
NV12 case in existing skl_plane_formats
- Added the 10bpc RGB formats

v4: Addressed review comments from Clinton A Taylor
"Why are we adding 10 bit RGB formats with the NV12 series patches?
Trying to set XR30 or AB30 results in error returned even though
the modes are advertised for the planes"
- Removed 10bit RGB formats added previously with NV12 series

v5: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.
Addressed review comments from Clinton A Taylor
"Why are we adding 10 bit RGB formats with the NV12 series patches?
Trying to set XR30 or AB30 results in error returned even though
the modes are advertised for the planes"
- Previous version has 10bit RGB format removed from VLV formats
by mistake. Fixing that in this version.
Removed 10bit RGB formats added previously with NV12 series
for SKL.

v6: Addressed review comments by Ville
Restricting the NV12 to BXT and PIPE A and B

v7: Rebased (me)

v8: Rebased (me)
Restricting NV12 changes to BXT and KBL
Restricting NV12 changes for plane 0 (overlay)

v9: Rebased (me)

v10: Addressed review comments from Maarten.
Adding NV12 to skl_plane_formats itself.

v11: Addressed review comments from Shashank Sharma

v12: Addressed review comments from Shashank Sharma
Made the condition in intel_sprite_plane_create
simple and easy to read as suggested.

v13: Adding reviewed by tag from Shashank Sharma
Addressed review comments from Juha-Pekka Heikkila
"NV12 not to be supported by SKL"

v14: Addressed review comments from Ville
Added skl_planar_formats to include NV12
and a check skl_plane_has_planar in sprite create
Added NV12 format to skl_mod_supported. These were
review comments from Kristian Høgsberg <hoegsberg@gmail.com>

v15: Added reviewed by from Juha-Pekka Heikkila

v16: Rebased the series

v17: Added all tiling under mod supported for NV12
Credits to Megha Aggarwal

v18: Added RB by Maarten and Kristian

Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c73553a..cdcae9e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
+static uint32_t skl_planar_formats[] = {
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
+};
+
 static const uint64_t skl_plane_format_modifiers_noccs[] = {
 	I915_FORMAT_MOD_Yf_TILED,
 	I915_FORMAT_MOD_Y_TILED,
@@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 		if (modifier == I915_FORMAT_MOD_Yf_TILED)
 			return true;
 		/* fall through */
+	case DRM_FORMAT_NV12:
+		if (modifier == DRM_FORMAT_MOD_LINEAR ||
+		    modifier == I915_FORMAT_MOD_X_TILED ||
+		    modifier == I915_FORMAT_MOD_Y_TILED ||
+		    modifier == I915_FORMAT_MOD_Yf_TILED)
+			return true;
 	case DRM_FORMAT_C8:
 		if (modifier == DRM_FORMAT_MOD_LINEAR ||
 		    modifier == I915_FORMAT_MOD_X_TILED ||
@@ -1373,8 +1392,14 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 		intel_plane->disable_plane = skl_disable_plane;
 		intel_plane->get_hw_state = skl_plane_get_hw_state;
 
-		plane_formats = skl_plane_formats;
-		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		if (skl_plane_has_planar(dev_priv, pipe,
+					 PLANE_SPRITE0 + plane)) {
+			plane_formats = skl_planar_formats;
+			num_plane_formats = ARRAY_SIZE(skl_planar_formats);
+		} else {
+			plane_formats = skl_plane_formats;
+			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		}
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
 			modifiers = skl_plane_format_modifiers_ccs;
-- 
2.7.4

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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Maarten Lankhorst]

Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> From: Chandra Konduru <chandra.konduru@intel.com>
>
> This patch adds NV12 to list of supported formats for sprite plane.
>
> v2: Rebased (me)
>
> v3: Review comments by Ville addressed
> - Removed skl_plane_formats_with_nv12 and added
> NV12 case in existing skl_plane_formats
> - Added the 10bpc RGB formats
>
> v4: Addressed review comments from Clinton A Taylor
> "Why are we adding 10 bit RGB formats with the NV12 series patches?
> Trying to set XR30 or AB30 results in error returned even though
> the modes are advertised for the planes"
> - Removed 10bit RGB formats added previously with NV12 series
>
> v5: Missed the Tested-by/Reviewed-by in the previous series
> Adding the same to commit message in this version.
> Addressed review comments from Clinton A Taylor
> "Why are we adding 10 bit RGB formats with the NV12 series patches?
> Trying to set XR30 or AB30 results in error returned even though
> the modes are advertised for the planes"
> - Previous version has 10bit RGB format removed from VLV formats
> by mistake. Fixing that in this version.
> Removed 10bit RGB formats added previously with NV12 series
> for SKL.
>
> v6: Addressed review comments by Ville
> Restricting the NV12 to BXT and PIPE A and B
>
> v7: Rebased (me)
>
> v8: Rebased (me)
> Restricting NV12 changes to BXT and KBL
> Restricting NV12 changes for plane 0 (overlay)
>
> v9: Rebased (me)
>
> v10: Addressed review comments from Maarten.
> Adding NV12 to skl_plane_formats itself.
>
> v11: Addressed review comments from Shashank Sharma
>
> v12: Addressed review comments from Shashank Sharma
> Made the condition in intel_sprite_plane_create
> simple and easy to read as suggested.
>
> v13: Adding reviewed by tag from Shashank Sharma
> Addressed review comments from Juha-Pekka Heikkila
> "NV12 not to be supported by SKL"
>
> v14: Addressed review comments from Ville
> Added skl_planar_formats to include NV12
> and a check skl_plane_has_planar in sprite create
> Added NV12 format to skl_mod_supported. These were
> review comments from Kristian Høgsberg <hoegsberg@gmail.com>
>
> v15: Added reviewed by from Juha-Pekka Heikkila
>
> v16: Rebased the series
>
> v17: Added all tiling under mod supported for NV12
> Credits to Megha Aggarwal
>
> v18: Added RB by Maarten and Kristian
>
> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c73553a..cdcae9e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> +static uint32_t skl_planar_formats[] = {
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_YUYV,
> +	DRM_FORMAT_YVYU,
> +	DRM_FORMAT_UYVY,
> +	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
> +};
> +
>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
>  	I915_FORMAT_MOD_Yf_TILED,
>  	I915_FORMAT_MOD_Y_TILED,
> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
>  			return true;
>  		/* fall through */
> +	case DRM_FORMAT_NV12:
> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> +		    modifier == I915_FORMAT_MOD_X_TILED ||
> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
> +			return true;
On patch 5 and 6: this is a tad overkill, just put it below DRM_FORMAT_VYUY and let it fall through. It's not different from the other formats. :)
Otherwise looks good, I'll wait until the changes from drm-misc-next are merged into drm-intel-next-queued then this series can be applied with the minor fixup.
>  	case DRM_FORMAT_C8:
>  		if (modifier == DRM_FORMAT_MOD_LINEAR ||
>  		    modifier == I915_FORMAT_MOD_X_TILED ||
> @@ -1373,8 +1392,14 @@ intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  		intel_plane->disable_plane = skl_disable_plane;
>  		intel_plane->get_hw_state = skl_plane_get_hw_state;
>  
> -		plane_formats = skl_plane_formats;
> -		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> +		if (skl_plane_has_planar(dev_priv, pipe,
> +					 PLANE_SPRITE0 + plane)) {
> +			plane_formats = skl_planar_formats;
> +			num_plane_formats = ARRAY_SIZE(skl_planar_formats);
> +		} else {
> +			plane_formats = skl_plane_formats;
> +			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> +		}
>  
>  		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 + plane))
>  			modifiers = skl_plane_format_modifiers_ccs;


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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Monday, May 7, 2018 1:38 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> > From: Chandra Konduru <chandra.konduru@intel.com>
> >
> > This patch adds NV12 to list of supported formats for sprite plane.
> >
> > v2: Rebased (me)
> >
> > v3: Review comments by Ville addressed
> > - Removed skl_plane_formats_with_nv12 and added
> > NV12 case in existing skl_plane_formats
> > - Added the 10bpc RGB formats
> >
> > v4: Addressed review comments from Clinton A Taylor "Why are we
> adding
> > 10 bit RGB formats with the NV12 series patches?
> > Trying to set XR30 or AB30 results in error returned even though the
> > modes are advertised for the planes"
> > - Removed 10bit RGB formats added previously with NV12 series
> >
> > v5: Missed the Tested-by/Reviewed-by in the previous series Adding the
> > same to commit message in this version.
> > Addressed review comments from Clinton A Taylor "Why are we adding 10
> > bit RGB formats with the NV12 series patches?
> > Trying to set XR30 or AB30 results in error returned even though the
> > modes are advertised for the planes"
> > - Previous version has 10bit RGB format removed from VLV formats by
> > mistake. Fixing that in this version.
> > Removed 10bit RGB formats added previously with NV12 series for SKL.
> >
> > v6: Addressed review comments by Ville Restricting the NV12 to BXT and
> > PIPE A and B
> >
> > v7: Rebased (me)
> >
> > v8: Rebased (me)
> > Restricting NV12 changes to BXT and KBL Restricting NV12 changes for
> > plane 0 (overlay)
> >
> > v9: Rebased (me)
> >
> > v10: Addressed review comments from Maarten.
> > Adding NV12 to skl_plane_formats itself.
> >
> > v11: Addressed review comments from Shashank Sharma
> >
> > v12: Addressed review comments from Shashank Sharma Made the
> condition
> > in intel_sprite_plane_create simple and easy to read as suggested.
> >
> > v13: Adding reviewed by tag from Shashank Sharma Addressed review
> > comments from Juha-Pekka Heikkila
> > "NV12 not to be supported by SKL"
> >
> > v14: Addressed review comments from Ville Added skl_planar_formats to
> > include NV12 and a check skl_plane_has_planar in sprite create Added
> > NV12 format to skl_mod_supported. These were review comments from
> > Kristian Høgsberg <hoegsberg@gmail.com>
> >
> > v15: Added reviewed by from Juha-Pekka Heikkila
> >
> > v16: Rebased the series
> >
> > v17: Added all tiling under mod supported for NV12 Credits to Megha
> > Aggarwal
> >
> > v18: Added RB by Maarten and Kristian
> >
> > Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
> > Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
> > Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> > Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> > Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 29
> > +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index c73553a..cdcae9e 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
> >  	DRM_FORMAT_VYUY,
> >  };
> >
> > +static uint32_t skl_planar_formats[] = {
> > +	DRM_FORMAT_RGB565,
> > +	DRM_FORMAT_ABGR8888,
> > +	DRM_FORMAT_ARGB8888,
> > +	DRM_FORMAT_XBGR8888,
> > +	DRM_FORMAT_XRGB8888,
> > +	DRM_FORMAT_YUYV,
> > +	DRM_FORMAT_YVYU,
> > +	DRM_FORMAT_UYVY,
> > +	DRM_FORMAT_VYUY,
> > +	DRM_FORMAT_NV12,
> > +};
> > +
> >  static const uint64_t skl_plane_format_modifiers_noccs[] = {
> >  	I915_FORMAT_MOD_Yf_TILED,
> >  	I915_FORMAT_MOD_Y_TILED,
> > @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
> format, uint64_t modifier)
> >  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> >  			return true;
> >  		/* fall through */
> > +	case DRM_FORMAT_NV12:
> > +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> > +		    modifier == I915_FORMAT_MOD_X_TILED ||
> > +		    modifier == I915_FORMAT_MOD_Y_TILED ||
> > +		    modifier == I915_FORMAT_MOD_Yf_TILED)
> > +			return true;
> On patch 5 and 6: this is a tad overkill, just put it below
> DRM_FORMAT_VYUY and let it fall through. It's not different from the other
> formats. :) Otherwise looks good, I'll wait until the changes from drm-misc-
> next are merged into drm-intel-next-queued then this series can be applied
> with the minor fixup.

Sure, thank you. I'll make the change. I did this to fix a review comment - which said all modifiers aren’t
Covered for NV12 - when we earlier had it under VYUV. Would that be still okay?

Regards
Vidya

> >  	case DRM_FORMAT_C8:
> >  		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> >  		    modifier == I915_FORMAT_MOD_X_TILED || @@ -1373,8
> +1392,14 @@
> > intel_sprite_plane_create(struct drm_i915_private *dev_priv,
> >  		intel_plane->disable_plane = skl_disable_plane;
> >  		intel_plane->get_hw_state = skl_plane_get_hw_state;
> >
> > -		plane_formats = skl_plane_formats;
> > -		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> > +		if (skl_plane_has_planar(dev_priv, pipe,
> > +					 PLANE_SPRITE0 + plane)) {
> > +			plane_formats = skl_planar_formats;
> > +			num_plane_formats =
> ARRAY_SIZE(skl_planar_formats);
> > +		} else {
> > +			plane_formats = skl_plane_formats;
> > +			num_plane_formats =
> ARRAY_SIZE(skl_plane_formats);
> > +		}
> >
> >  		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0 +
> plane))
> >  			modifiers = skl_plane_format_modifiers_ccs;
> 

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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Maarten Lankhorst]

Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Monday, May 7, 2018 1:38 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
>> format for sprite plane
>>
>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
>>> From: Chandra Konduru <chandra.konduru@intel.com>
>>>
>>> This patch adds NV12 to list of supported formats for sprite plane.
>>>
>>> v2: Rebased (me)
>>>
>>> v3: Review comments by Ville addressed
>>> - Removed skl_plane_formats_with_nv12 and added
>>> NV12 case in existing skl_plane_formats
>>> - Added the 10bpc RGB formats
>>>
>>> v4: Addressed review comments from Clinton A Taylor "Why are we
>> adding
>>> 10 bit RGB formats with the NV12 series patches?
>>> Trying to set XR30 or AB30 results in error returned even though the
>>> modes are advertised for the planes"
>>> - Removed 10bit RGB formats added previously with NV12 series
>>>
>>> v5: Missed the Tested-by/Reviewed-by in the previous series Adding the
>>> same to commit message in this version.
>>> Addressed review comments from Clinton A Taylor "Why are we adding 10
>>> bit RGB formats with the NV12 series patches?
>>> Trying to set XR30 or AB30 results in error returned even though the
>>> modes are advertised for the planes"
>>> - Previous version has 10bit RGB format removed from VLV formats by
>>> mistake. Fixing that in this version.
>>> Removed 10bit RGB formats added previously with NV12 series for SKL.
>>>
>>> v6: Addressed review comments by Ville Restricting the NV12 to BXT and
>>> PIPE A and B
>>>
>>> v7: Rebased (me)
>>>
>>> v8: Rebased (me)
>>> Restricting NV12 changes to BXT and KBL Restricting NV12 changes for
>>> plane 0 (overlay)
>>>
>>> v9: Rebased (me)
>>>
>>> v10: Addressed review comments from Maarten.
>>> Adding NV12 to skl_plane_formats itself.
>>>
>>> v11: Addressed review comments from Shashank Sharma
>>>
>>> v12: Addressed review comments from Shashank Sharma Made the
>> condition
>>> in intel_sprite_plane_create simple and easy to read as suggested.
>>>
>>> v13: Adding reviewed by tag from Shashank Sharma Addressed review
>>> comments from Juha-Pekka Heikkila
>>> "NV12 not to be supported by SKL"
>>>
>>> v14: Addressed review comments from Ville Added skl_planar_formats to
>>> include NV12 and a check skl_plane_has_planar in sprite create Added
>>> NV12 format to skl_mod_supported. These were review comments from
>>> Kristian Høgsberg <hoegsberg@gmail.com>
>>>
>>> v15: Added reviewed by from Juha-Pekka Heikkila
>>>
>>> v16: Rebased the series
>>>
>>> v17: Added all tiling under mod supported for NV12 Credits to Megha
>>> Aggarwal
>>>
>>> v18: Added RB by Maarten and Kristian
>>>
>>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
>>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
>>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
>>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_sprite.c | 29
>>> +++++++++++++++++++++++++++--
>>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>> index c73553a..cdcae9e 100644
>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
>>>  	DRM_FORMAT_VYUY,
>>>  };
>>>
>>> +static uint32_t skl_planar_formats[] = {
>>> +	DRM_FORMAT_RGB565,
>>> +	DRM_FORMAT_ABGR8888,
>>> +	DRM_FORMAT_ARGB8888,
>>> +	DRM_FORMAT_XBGR8888,
>>> +	DRM_FORMAT_XRGB8888,
>>> +	DRM_FORMAT_YUYV,
>>> +	DRM_FORMAT_YVYU,
>>> +	DRM_FORMAT_UYVY,
>>> +	DRM_FORMAT_VYUY,
>>> +	DRM_FORMAT_NV12,
>>> +};
>>> +
>>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
>>>  	I915_FORMAT_MOD_Yf_TILED,
>>>  	I915_FORMAT_MOD_Y_TILED,
>>> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
>> format, uint64_t modifier)
>>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
>>>  			return true;
>>>  		/* fall through */
>>> +	case DRM_FORMAT_NV12:
>>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
>>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
>>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
>>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
>>> +			return true;
>> On patch 5 and 6: this is a tad overkill, just put it below
>> DRM_FORMAT_VYUY and let it fall through. It's not different from the other
>> formats. :) Otherwise looks good, I'll wait until the changes from drm-misc-
>> next are merged into drm-intel-next-queued then this series can be applied
>> with the minor fixup.
> Sure, thank you. I'll make the change. I did this to fix a review comment - which said all modifiers aren’t
> Covered for NV12 - when we earlier had it under VYUV. Would that be still okay?
Seems only CCS isn't covered, so putting it there should be fine. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Monday, May 7, 2018 1:44 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Monday, May 7, 2018 1:38 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >> supported format for sprite plane
> >>
> >> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> >>> From: Chandra Konduru <chandra.konduru@intel.com>
> >>>
> >>> This patch adds NV12 to list of supported formats for sprite plane.
> >>>
> >>> v2: Rebased (me)
> >>>
> >>> v3: Review comments by Ville addressed
> >>> - Removed skl_plane_formats_with_nv12 and added
> >>> NV12 case in existing skl_plane_formats
> >>> - Added the 10bpc RGB formats
> >>>
> >>> v4: Addressed review comments from Clinton A Taylor "Why are we
> >> adding
> >>> 10 bit RGB formats with the NV12 series patches?
> >>> Trying to set XR30 or AB30 results in error returned even though the
> >>> modes are advertised for the planes"
> >>> - Removed 10bit RGB formats added previously with NV12 series
> >>>
> >>> v5: Missed the Tested-by/Reviewed-by in the previous series Adding
> >>> the same to commit message in this version.
> >>> Addressed review comments from Clinton A Taylor "Why are we adding
> >>> 10 bit RGB formats with the NV12 series patches?
> >>> Trying to set XR30 or AB30 results in error returned even though the
> >>> modes are advertised for the planes"
> >>> - Previous version has 10bit RGB format removed from VLV formats by
> >>> mistake. Fixing that in this version.
> >>> Removed 10bit RGB formats added previously with NV12 series for SKL.
> >>>
> >>> v6: Addressed review comments by Ville Restricting the NV12 to BXT
> >>> and PIPE A and B
> >>>
> >>> v7: Rebased (me)
> >>>
> >>> v8: Rebased (me)
> >>> Restricting NV12 changes to BXT and KBL Restricting NV12 changes for
> >>> plane 0 (overlay)
> >>>
> >>> v9: Rebased (me)
> >>>
> >>> v10: Addressed review comments from Maarten.
> >>> Adding NV12 to skl_plane_formats itself.
> >>>
> >>> v11: Addressed review comments from Shashank Sharma
> >>>
> >>> v12: Addressed review comments from Shashank Sharma Made the
> >> condition
> >>> in intel_sprite_plane_create simple and easy to read as suggested.
> >>>
> >>> v13: Adding reviewed by tag from Shashank Sharma Addressed review
> >>> comments from Juha-Pekka Heikkila
> >>> "NV12 not to be supported by SKL"
> >>>
> >>> v14: Addressed review comments from Ville Added skl_planar_formats
> >>> to include NV12 and a check skl_plane_has_planar in sprite create
> >>> Added
> >>> NV12 format to skl_mod_supported. These were review comments
> from
> >>> Kristian Høgsberg <hoegsberg@gmail.com>
> >>>
> >>> v15: Added reviewed by from Juha-Pekka Heikkila
> >>>
> >>> v16: Rebased the series
> >>>
> >>> v17: Added all tiling under mod supported for NV12 Credits to Megha
> >>> Aggarwal
> >>>
> >>> v18: Added RB by Maarten and Kristian
> >>>
> >>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
> >>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
> >>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
> >>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> >>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> >>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> >>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_sprite.c | 29
> >>> +++++++++++++++++++++++++++--
> >>>  1 file changed, 27 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>> index c73553a..cdcae9e 100644
> >>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
> >>>  	DRM_FORMAT_VYUY,
> >>>  };
> >>>
> >>> +static uint32_t skl_planar_formats[] = {
> >>> +	DRM_FORMAT_RGB565,
> >>> +	DRM_FORMAT_ABGR8888,
> >>> +	DRM_FORMAT_ARGB8888,
> >>> +	DRM_FORMAT_XBGR8888,
> >>> +	DRM_FORMAT_XRGB8888,
> >>> +	DRM_FORMAT_YUYV,
> >>> +	DRM_FORMAT_YVYU,
> >>> +	DRM_FORMAT_UYVY,
> >>> +	DRM_FORMAT_VYUY,
> >>> +	DRM_FORMAT_NV12,
> >>> +};
> >>> +
> >>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
> >>>  	I915_FORMAT_MOD_Yf_TILED,
> >>>  	I915_FORMAT_MOD_Y_TILED,
> >>> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
> >> format, uint64_t modifier)
> >>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>  			return true;
> >>>  		/* fall through */
> >>> +	case DRM_FORMAT_NV12:
> >>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> >>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
> >>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
> >>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
> >>> +			return true;
> >> On patch 5 and 6: this is a tad overkill, just put it below
> >> DRM_FORMAT_VYUY and let it fall through. It's not different from the
> >> other formats. :) Otherwise looks good, I'll wait until the changes
> >> from drm-misc- next are merged into drm-intel-next-queued then this
> >> series can be applied with the minor fixup.
> > Sure, thank you. I'll make the change. I did this to fix a review
> > comment - which said all modifiers aren’t Covered for NV12 - when we
> earlier had it under VYUV. Would that be still okay?
> Seems only CCS isn't covered, so putting it there should be fine. :)
Oh okay - sure thank you. Will make the change and float the series.

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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Srinivas, Vidya]

> -----Original Message-----
> From: Srinivas, Vidya
> Sent: Monday, May 7, 2018 1:46 PM
> To: 'Maarten Lankhorst' <maarten.lankhorst@linux.intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> 
> 
> > -----Original Message-----
> > From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> > Sent: Monday, May 7, 2018 1:44 PM
> > To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> > supported format for sprite plane
> >
> > Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
> > >
> > >> -----Original Message-----
> > >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> > >> Sent: Monday, May 7, 2018 1:38 PM
> > >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> > >> gfx@lists.freedesktop.org
> > >> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> > >> supported format for sprite plane
> > >>
> > >> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> > >>> From: Chandra Konduru <chandra.konduru@intel.com>
> > >>>
> > >>> This patch adds NV12 to list of supported formats for sprite plane.
> > >>>
> > >>> v2: Rebased (me)
> > >>>
> > >>> v3: Review comments by Ville addressed
> > >>> - Removed skl_plane_formats_with_nv12 and added
> > >>> NV12 case in existing skl_plane_formats
> > >>> - Added the 10bpc RGB formats
> > >>>
> > >>> v4: Addressed review comments from Clinton A Taylor "Why are we
> > >> adding
> > >>> 10 bit RGB formats with the NV12 series patches?
> > >>> Trying to set XR30 or AB30 results in error returned even though
> > >>> the modes are advertised for the planes"
> > >>> - Removed 10bit RGB formats added previously with NV12 series
> > >>>
> > >>> v5: Missed the Tested-by/Reviewed-by in the previous series Adding
> > >>> the same to commit message in this version.
> > >>> Addressed review comments from Clinton A Taylor "Why are we
> adding
> > >>> 10 bit RGB formats with the NV12 series patches?
> > >>> Trying to set XR30 or AB30 results in error returned even though
> > >>> the modes are advertised for the planes"
> > >>> - Previous version has 10bit RGB format removed from VLV formats
> > >>> by mistake. Fixing that in this version.
> > >>> Removed 10bit RGB formats added previously with NV12 series for
> SKL.
> > >>>
> > >>> v6: Addressed review comments by Ville Restricting the NV12 to BXT
> > >>> and PIPE A and B
> > >>>
> > >>> v7: Rebased (me)
> > >>>
> > >>> v8: Rebased (me)
> > >>> Restricting NV12 changes to BXT and KBL Restricting NV12 changes
> > >>> for plane 0 (overlay)
> > >>>
> > >>> v9: Rebased (me)
> > >>>
> > >>> v10: Addressed review comments from Maarten.
> > >>> Adding NV12 to skl_plane_formats itself.
> > >>>
> > >>> v11: Addressed review comments from Shashank Sharma
> > >>>
> > >>> v12: Addressed review comments from Shashank Sharma Made the
> > >> condition
> > >>> in intel_sprite_plane_create simple and easy to read as suggested.
> > >>>
> > >>> v13: Adding reviewed by tag from Shashank Sharma Addressed review
> > >>> comments from Juha-Pekka Heikkila
> > >>> "NV12 not to be supported by SKL"
> > >>>
> > >>> v14: Addressed review comments from Ville Added
> skl_planar_formats
> > >>> to include NV12 and a check skl_plane_has_planar in sprite create
> > >>> Added
> > >>> NV12 format to skl_mod_supported. These were review comments
> > from
> > >>> Kristian Høgsberg <hoegsberg@gmail.com>
> > >>>
> > >>> v15: Added reviewed by from Juha-Pekka Heikkila
> > >>>
> > >>> v16: Rebased the series
> > >>>
> > >>> v17: Added all tiling under mod supported for NV12 Credits to
> > >>> Megha Aggarwal
> > >>>
> > >>> v18: Added RB by Maarten and Kristian
> > >>>
> > >>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
> > >>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
> > >>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
> > >>> Reviewed-by: Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>
> > >>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> > >>> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > >>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> > >>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> > >>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > >>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> > >>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > >>> ---
> > >>>  drivers/gpu/drm/i915/intel_sprite.c | 29
> > >>> +++++++++++++++++++++++++++--
> > >>>  1 file changed, 27 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > >>> b/drivers/gpu/drm/i915/intel_sprite.c
> > >>> index c73553a..cdcae9e 100644
> > >>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> > >>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > >>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
> > >>>  	DRM_FORMAT_VYUY,
> > >>>  };
> > >>>
> > >>> +static uint32_t skl_planar_formats[] = {
> > >>> +	DRM_FORMAT_RGB565,
> > >>> +	DRM_FORMAT_ABGR8888,
> > >>> +	DRM_FORMAT_ARGB8888,
> > >>> +	DRM_FORMAT_XBGR8888,
> > >>> +	DRM_FORMAT_XRGB8888,
> > >>> +	DRM_FORMAT_YUYV,
> > >>> +	DRM_FORMAT_YVYU,
> > >>> +	DRM_FORMAT_UYVY,
> > >>> +	DRM_FORMAT_VYUY,
> > >>> +	DRM_FORMAT_NV12,
> > >>> +};
> > >>> +
> > >>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
> > >>>  	I915_FORMAT_MOD_Yf_TILED,
> > >>>  	I915_FORMAT_MOD_Y_TILED,
> > >>> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
> > >> format, uint64_t modifier)
> > >>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> > >>>  			return true;
> > >>>  		/* fall through */
> > >>> +	case DRM_FORMAT_NV12:
> > >>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> > >>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
> > >>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
> > >>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
> > >>> +			return true;
> > >> On patch 5 and 6: this is a tad overkill, just put it below
> > >> DRM_FORMAT_VYUY and let it fall through. It's not different from
> > >> the other formats. :) Otherwise looks good, I'll wait until the
> > >> changes from drm-misc- next are merged into drm-intel-next-queued
> > >> then this series can be applied with the minor fixup.
> > > Sure, thank you. I'll make the change. I did this to fix a review
> > > comment - which said all modifiers aren’t Covered for NV12 - when we
> > earlier had it under VYUV. Would that be still okay?
> > Seems only CCS isn't covered, so putting it there should be fine. :)
> Oh okay - sure thank you. Will make the change and float the series.
Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we need to
add the other modifiers like MOD_LINEAR, MOD_X_TILED and MOD_Y_TILED?
> 
> Regards
> Vidya
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Maarten Lankhorst]

Op 07-05-18 om 10:20 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Srinivas, Vidya
>> Sent: Monday, May 7, 2018 1:46 PM
>> To: 'Maarten Lankhorst' <maarten.lankhorst@linux.intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
>> format for sprite plane
>>
>>
>>
>>> -----Original Message-----
>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>> Sent: Monday, May 7, 2018 1:44 PM
>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>> gfx@lists.freedesktop.org
>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>> supported format for sprite plane
>>>
>>> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
>>>>> -----Original Message-----
>>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>>> Sent: Monday, May 7, 2018 1:38 PM
>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>>> gfx@lists.freedesktop.org
>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>>> supported format for sprite plane
>>>>>
>>>>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
>>>>>> From: Chandra Konduru <chandra.konduru@intel.com>
>>>>>>
>>>>>> This patch adds NV12 to list of supported formats for sprite plane.
>>>>>>
>>>>>> v2: Rebased (me)
>>>>>>
>>>>>> v3: Review comments by Ville addressed
>>>>>> - Removed skl_plane_formats_with_nv12 and added
>>>>>> NV12 case in existing skl_plane_formats
>>>>>> - Added the 10bpc RGB formats
>>>>>>
>>>>>> v4: Addressed review comments from Clinton A Taylor "Why are we
>>>>> adding
>>>>>> 10 bit RGB formats with the NV12 series patches?
>>>>>> Trying to set XR30 or AB30 results in error returned even though
>>>>>> the modes are advertised for the planes"
>>>>>> - Removed 10bit RGB formats added previously with NV12 series
>>>>>>
>>>>>> v5: Missed the Tested-by/Reviewed-by in the previous series Adding
>>>>>> the same to commit message in this version.
>>>>>> Addressed review comments from Clinton A Taylor "Why are we
>> adding
>>>>>> 10 bit RGB formats with the NV12 series patches?
>>>>>> Trying to set XR30 or AB30 results in error returned even though
>>>>>> the modes are advertised for the planes"
>>>>>> - Previous version has 10bit RGB format removed from VLV formats
>>>>>> by mistake. Fixing that in this version.
>>>>>> Removed 10bit RGB formats added previously with NV12 series for
>> SKL.
>>>>>> v6: Addressed review comments by Ville Restricting the NV12 to BXT
>>>>>> and PIPE A and B
>>>>>>
>>>>>> v7: Rebased (me)
>>>>>>
>>>>>> v8: Rebased (me)
>>>>>> Restricting NV12 changes to BXT and KBL Restricting NV12 changes
>>>>>> for plane 0 (overlay)
>>>>>>
>>>>>> v9: Rebased (me)
>>>>>>
>>>>>> v10: Addressed review comments from Maarten.
>>>>>> Adding NV12 to skl_plane_formats itself.
>>>>>>
>>>>>> v11: Addressed review comments from Shashank Sharma
>>>>>>
>>>>>> v12: Addressed review comments from Shashank Sharma Made the
>>>>> condition
>>>>>> in intel_sprite_plane_create simple and easy to read as suggested.
>>>>>>
>>>>>> v13: Adding reviewed by tag from Shashank Sharma Addressed review
>>>>>> comments from Juha-Pekka Heikkila
>>>>>> "NV12 not to be supported by SKL"
>>>>>>
>>>>>> v14: Addressed review comments from Ville Added
>> skl_planar_formats
>>>>>> to include NV12 and a check skl_plane_has_planar in sprite create
>>>>>> Added
>>>>>> NV12 format to skl_mod_supported. These were review comments
>>> from
>>>>>> Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>>
>>>>>> v15: Added reviewed by from Juha-Pekka Heikkila
>>>>>>
>>>>>> v16: Rebased the series
>>>>>>
>>>>>> v17: Added all tiling under mod supported for NV12 Credits to
>>>>>> Megha Aggarwal
>>>>>>
>>>>>> v18: Added RB by Maarten and Kristian
>>>>>>
>>>>>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
>>>>>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>> Reviewed-by: Maarten Lankhorst
>> <maarten.lankhorst@linux.intel.com>
>>>>>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>>>>> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>>>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>>>>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>>>>> ---
>>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 29
>>>>>> +++++++++++++++++++++++++++--
>>>>>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>> index c73553a..cdcae9e 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
>>>>>>  	DRM_FORMAT_VYUY,
>>>>>>  };
>>>>>>
>>>>>> +static uint32_t skl_planar_formats[] = {
>>>>>> +	DRM_FORMAT_RGB565,
>>>>>> +	DRM_FORMAT_ABGR8888,
>>>>>> +	DRM_FORMAT_ARGB8888,
>>>>>> +	DRM_FORMAT_XBGR8888,
>>>>>> +	DRM_FORMAT_XRGB8888,
>>>>>> +	DRM_FORMAT_YUYV,
>>>>>> +	DRM_FORMAT_YVYU,
>>>>>> +	DRM_FORMAT_UYVY,
>>>>>> +	DRM_FORMAT_VYUY,
>>>>>> +	DRM_FORMAT_NV12,
>>>>>> +};
>>>>>> +
>>>>>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
>>>>>>  	I915_FORMAT_MOD_Yf_TILED,
>>>>>>  	I915_FORMAT_MOD_Y_TILED,
>>>>>> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
>>>>> format, uint64_t modifier)
>>>>>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
>>>>>>  			return true;
>>>>>>  		/* fall through */
>>>>>> +	case DRM_FORMAT_NV12:
>>>>>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
>>>>>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
>>>>>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
>>>>>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
>>>>>> +			return true;
>>>>> On patch 5 and 6: this is a tad overkill, just put it below
>>>>> DRM_FORMAT_VYUY and let it fall through. It's not different from
>>>>> the other formats. :) Otherwise looks good, I'll wait until the
>>>>> changes from drm-misc- next are merged into drm-intel-next-queued
>>>>> then this series can be applied with the minor fixup.
>>>> Sure, thank you. I'll make the change. I did this to fix a review
>>>> comment - which said all modifiers aren’t Covered for NV12 - when we
>>> earlier had it under VYUV. Would that be still okay?
>>> Seems only CCS isn't covered, so putting it there should be fine. :)
>> Oh okay - sure thank you. Will make the change and float the series.
> Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we need to
> add the other modifiers like MOD_LINEAR, MOD_X_TILED and MOD_Y_TILED?
>> Regards
>> Vidya

This is a C switch, unless there is a break it will continue, so it works as intended. :)

For example format UYVY if modifier == Yf_TILED_CCS, it will ignore all 
the lines with case and default, and continue to the end of the switch 
statement, or until it hits a 'break' so the below:


	case DRM_FORMAT_UYVY:
	case DRM_FORMAT_VYUY:
		if (modifier == I915_FORMAT_MOD_Yf_TILED)
			return true;
		/* fall through */
	case DRM_FORMAT_C8:
		if (modifier == DRM_FORMAT_MOD_LINEAR ||
		    modifier == I915_FORMAT_MOD_X_TILED ||
		    modifier == I915_FORMAT_MOD_Y_TILED)
			return true;
		/* fall through */
	default:
		return false;
}

for UYVY ends up being the same as:

if (format == DRM_FORMAT_UYVY) {
	if (modifier == I915_FORMAT_MOD_Yf_TILED)
		return true;
	if (modifier == DRM_FORMAT_MOD_LINEAR ||
	    modifier == I915_FORMAT_MOD_X_TILED ||
	    modifier == I915_FORMAT_MOD_Y_TILED)
		return true;
	return false;
}

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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Monday, May 7, 2018 1:55 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> Op 07-05-18 om 10:20 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Srinivas, Vidya
> >> Sent: Monday, May 7, 2018 1:46 PM
> >> To: 'Maarten Lankhorst' <maarten.lankhorst@linux.intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >> supported format for sprite plane
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >>> Sent: Monday, May 7, 2018 1:44 PM
> >>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>> gfx@lists.freedesktop.org
> >>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>> supported format for sprite plane
> >>>
> >>> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
> >>>>> -----Original Message-----
> >>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >>>>> Sent: Monday, May 7, 2018 1:38 PM
> >>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>>> gfx@lists.freedesktop.org
> >>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>>> supported format for sprite plane
> >>>>>
> >>>>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> >>>>>> From: Chandra Konduru <chandra.konduru@intel.com>
> >>>>>>
> >>>>>> This patch adds NV12 to list of supported formats for sprite plane.
> >>>>>>
> >>>>>> v2: Rebased (me)
> >>>>>>
> >>>>>> v3: Review comments by Ville addressed
> >>>>>> - Removed skl_plane_formats_with_nv12 and added
> >>>>>> NV12 case in existing skl_plane_formats
> >>>>>> - Added the 10bpc RGB formats
> >>>>>>
> >>>>>> v4: Addressed review comments from Clinton A Taylor "Why are we
> >>>>> adding
> >>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>> Trying to set XR30 or AB30 results in error returned even though
> >>>>>> the modes are advertised for the planes"
> >>>>>> - Removed 10bit RGB formats added previously with NV12 series
> >>>>>>
> >>>>>> v5: Missed the Tested-by/Reviewed-by in the previous series
> >>>>>> Adding the same to commit message in this version.
> >>>>>> Addressed review comments from Clinton A Taylor "Why are we
> >> adding
> >>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>> Trying to set XR30 or AB30 results in error returned even though
> >>>>>> the modes are advertised for the planes"
> >>>>>> - Previous version has 10bit RGB format removed from VLV formats
> >>>>>> by mistake. Fixing that in this version.
> >>>>>> Removed 10bit RGB formats added previously with NV12 series for
> >> SKL.
> >>>>>> v6: Addressed review comments by Ville Restricting the NV12 to
> >>>>>> BXT and PIPE A and B
> >>>>>>
> >>>>>> v7: Rebased (me)
> >>>>>>
> >>>>>> v8: Rebased (me)
> >>>>>> Restricting NV12 changes to BXT and KBL Restricting NV12 changes
> >>>>>> for plane 0 (overlay)
> >>>>>>
> >>>>>> v9: Rebased (me)
> >>>>>>
> >>>>>> v10: Addressed review comments from Maarten.
> >>>>>> Adding NV12 to skl_plane_formats itself.
> >>>>>>
> >>>>>> v11: Addressed review comments from Shashank Sharma
> >>>>>>
> >>>>>> v12: Addressed review comments from Shashank Sharma Made the
> >>>>> condition
> >>>>>> in intel_sprite_plane_create simple and easy to read as suggested.
> >>>>>>
> >>>>>> v13: Adding reviewed by tag from Shashank Sharma Addressed
> review
> >>>>>> comments from Juha-Pekka Heikkila
> >>>>>> "NV12 not to be supported by SKL"
> >>>>>>
> >>>>>> v14: Addressed review comments from Ville Added
> >> skl_planar_formats
> >>>>>> to include NV12 and a check skl_plane_has_planar in sprite create
> >>>>>> Added
> >>>>>> NV12 format to skl_mod_supported. These were review comments
> >>> from
> >>>>>> Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>>
> >>>>>> v15: Added reviewed by from Juha-Pekka Heikkila
> >>>>>>
> >>>>>> v16: Rebased the series
> >>>>>>
> >>>>>> v17: Added all tiling under mod supported for NV12 Credits to
> >>>>>> Megha Aggarwal
> >>>>>>
> >>>>>> v18: Added RB by Maarten and Kristian
> >>>>>>
> >>>>>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
> >>>>>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>> Reviewed-by: Maarten Lankhorst
> >> <maarten.lankhorst@linux.intel.com>
> >>>>>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>>>>> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >>>>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>>>>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> >>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> >>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>>>>> ---
> >>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 29
> >>>>>> +++++++++++++++++++++++++++--
> >>>>>>  1 file changed, 27 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> index c73553a..cdcae9e 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
> >>>>>>  	DRM_FORMAT_VYUY,
> >>>>>>  };
> >>>>>>
> >>>>>> +static uint32_t skl_planar_formats[] = {
> >>>>>> +	DRM_FORMAT_RGB565,
> >>>>>> +	DRM_FORMAT_ABGR8888,
> >>>>>> +	DRM_FORMAT_ARGB8888,
> >>>>>> +	DRM_FORMAT_XBGR8888,
> >>>>>> +	DRM_FORMAT_XRGB8888,
> >>>>>> +	DRM_FORMAT_YUYV,
> >>>>>> +	DRM_FORMAT_YVYU,
> >>>>>> +	DRM_FORMAT_UYVY,
> >>>>>> +	DRM_FORMAT_VYUY,
> >>>>>> +	DRM_FORMAT_NV12,
> >>>>>> +};
> >>>>>> +
> >>>>>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
> >>>>>>  	I915_FORMAT_MOD_Yf_TILED,
> >>>>>>  	I915_FORMAT_MOD_Y_TILED,
> >>>>>> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
> >>>>> format, uint64_t modifier)
> >>>>>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>>  			return true;
> >>>>>>  		/* fall through */
> >>>>>> +	case DRM_FORMAT_NV12:
> >>>>>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> >>>>>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
> >>>>>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
> >>>>>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>> +			return true;
> >>>>> On patch 5 and 6: this is a tad overkill, just put it below
> >>>>> DRM_FORMAT_VYUY and let it fall through. It's not different from
> >>>>> the other formats. :) Otherwise looks good, I'll wait until the
> >>>>> changes from drm-misc- next are merged into drm-intel-next-queued
> >>>>> then this series can be applied with the minor fixup.
> >>>> Sure, thank you. I'll make the change. I did this to fix a review
> >>>> comment - which said all modifiers aren’t Covered for NV12 - when
> >>>> we
> >>> earlier had it under VYUV. Would that be still okay?
> >>> Seems only CCS isn't covered, so putting it there should be fine. :)
> >> Oh okay - sure thank you. Will make the change and float the series.
> > Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we need to
> > add the other modifiers like MOD_LINEAR, MOD_X_TILED and
> MOD_Y_TILED?
> >> Regards
> >> Vidya
> 
> This is a C switch, unless there is a break it will continue, so it works as
> intended. :)
> 
> For example format UYVY if modifier == Yf_TILED_CCS, it will ignore all the
> lines with case and default, and continue to the end of the switch
> statement, or until it hits a 'break' so the below:
I am so sorry, I did not notice there was no break :( Got confused. Thank you.

Regards
Vidya
> 
> 
> 	case DRM_FORMAT_UYVY:
> 	case DRM_FORMAT_VYUY:
> 		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> 			return true;
> 		/* fall through */
> 	case DRM_FORMAT_C8:
> 		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> 		    modifier == I915_FORMAT_MOD_X_TILED ||
> 		    modifier == I915_FORMAT_MOD_Y_TILED)
> 			return true;
> 		/* fall through */
> 	default:
> 		return false;
> }
> 
> for UYVY ends up being the same as:
> 
> if (format == DRM_FORMAT_UYVY) {
> 	if (modifier == I915_FORMAT_MOD_Yf_TILED)
> 		return true;
> 	if (modifier == DRM_FORMAT_MOD_LINEAR ||
> 	    modifier == I915_FORMAT_MOD_X_TILED ||
> 	    modifier == I915_FORMAT_MOD_Y_TILED)
> 		return true;
> 	return false;
> }

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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Maarten Lankhorst]

Op 07-05-18 om 10:29 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Monday, May 7, 2018 1:55 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
>> format for sprite plane
>>
>> Op 07-05-18 om 10:20 schreef Srinivas, Vidya:
>>>> -----Original Message-----
>>>> From: Srinivas, Vidya
>>>> Sent: Monday, May 7, 2018 1:46 PM
>>>> To: 'Maarten Lankhorst' <maarten.lankhorst@linux.intel.com>; intel-
>>>> gfx@lists.freedesktop.org
>>>> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>> supported format for sprite plane
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>>> Sent: Monday, May 7, 2018 1:44 PM
>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>>> gfx@lists.freedesktop.org
>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>>> supported format for sprite plane
>>>>>
>>>>> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
>>>>>>> -----Original Message-----
>>>>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>>>>> Sent: Monday, May 7, 2018 1:38 PM
>>>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>>>>> gfx@lists.freedesktop.org
>>>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>>>>> supported format for sprite plane
>>>>>>>
>>>>>>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
>>>>>>>> From: Chandra Konduru <chandra.konduru@intel.com>
>>>>>>>>
>>>>>>>> This patch adds NV12 to list of supported formats for sprite plane.
>>>>>>>>
>>>>>>>> v2: Rebased (me)
>>>>>>>>
>>>>>>>> v3: Review comments by Ville addressed
>>>>>>>> - Removed skl_plane_formats_with_nv12 and added
>>>>>>>> NV12 case in existing skl_plane_formats
>>>>>>>> - Added the 10bpc RGB formats
>>>>>>>>
>>>>>>>> v4: Addressed review comments from Clinton A Taylor "Why are we
>>>>>>> adding
>>>>>>>> 10 bit RGB formats with the NV12 series patches?
>>>>>>>> Trying to set XR30 or AB30 results in error returned even though
>>>>>>>> the modes are advertised for the planes"
>>>>>>>> - Removed 10bit RGB formats added previously with NV12 series
>>>>>>>>
>>>>>>>> v5: Missed the Tested-by/Reviewed-by in the previous series
>>>>>>>> Adding the same to commit message in this version.
>>>>>>>> Addressed review comments from Clinton A Taylor "Why are we
>>>> adding
>>>>>>>> 10 bit RGB formats with the NV12 series patches?
>>>>>>>> Trying to set XR30 or AB30 results in error returned even though
>>>>>>>> the modes are advertised for the planes"
>>>>>>>> - Previous version has 10bit RGB format removed from VLV formats
>>>>>>>> by mistake. Fixing that in this version.
>>>>>>>> Removed 10bit RGB formats added previously with NV12 series for
>>>> SKL.
>>>>>>>> v6: Addressed review comments by Ville Restricting the NV12 to
>>>>>>>> BXT and PIPE A and B
>>>>>>>>
>>>>>>>> v7: Rebased (me)
>>>>>>>>
>>>>>>>> v8: Rebased (me)
>>>>>>>> Restricting NV12 changes to BXT and KBL Restricting NV12 changes
>>>>>>>> for plane 0 (overlay)
>>>>>>>>
>>>>>>>> v9: Rebased (me)
>>>>>>>>
>>>>>>>> v10: Addressed review comments from Maarten.
>>>>>>>> Adding NV12 to skl_plane_formats itself.
>>>>>>>>
>>>>>>>> v11: Addressed review comments from Shashank Sharma
>>>>>>>>
>>>>>>>> v12: Addressed review comments from Shashank Sharma Made the
>>>>>>> condition
>>>>>>>> in intel_sprite_plane_create simple and easy to read as suggested.
>>>>>>>>
>>>>>>>> v13: Adding reviewed by tag from Shashank Sharma Addressed
>> review
>>>>>>>> comments from Juha-Pekka Heikkila
>>>>>>>> "NV12 not to be supported by SKL"
>>>>>>>>
>>>>>>>> v14: Addressed review comments from Ville Added
>>>> skl_planar_formats
>>>>>>>> to include NV12 and a check skl_plane_has_planar in sprite create
>>>>>>>> Added
>>>>>>>> NV12 format to skl_mod_supported. These were review comments
>>>>> from
>>>>>>>> Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>>>>
>>>>>>>> v15: Added reviewed by from Juha-Pekka Heikkila
>>>>>>>>
>>>>>>>> v16: Rebased the series
>>>>>>>>
>>>>>>>> v17: Added all tiling under mod supported for NV12 Credits to
>>>>>>>> Megha Aggarwal
>>>>>>>>
>>>>>>>> v18: Added RB by Maarten and Kristian
>>>>>>>>
>>>>>>>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
>>>>>>>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>>>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>>>> Reviewed-by: Maarten Lankhorst
>>>> <maarten.lankhorst@linux.intel.com>
>>>>>>>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>>>>>>> Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>>>>>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>>>>>>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>>>>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>>>>>>> ---
>>>>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 29
>>>>>>>> +++++++++++++++++++++++++++--
>>>>>>>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>> index c73553a..cdcae9e 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] = {
>>>>>>>>  	DRM_FORMAT_VYUY,
>>>>>>>>  };
>>>>>>>>
>>>>>>>> +static uint32_t skl_planar_formats[] = {
>>>>>>>> +	DRM_FORMAT_RGB565,
>>>>>>>> +	DRM_FORMAT_ABGR8888,
>>>>>>>> +	DRM_FORMAT_ARGB8888,
>>>>>>>> +	DRM_FORMAT_XBGR8888,
>>>>>>>> +	DRM_FORMAT_XRGB8888,
>>>>>>>> +	DRM_FORMAT_YUYV,
>>>>>>>> +	DRM_FORMAT_YVYU,
>>>>>>>> +	DRM_FORMAT_UYVY,
>>>>>>>> +	DRM_FORMAT_VYUY,
>>>>>>>> +	DRM_FORMAT_NV12,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
>>>>>>>>  	I915_FORMAT_MOD_Yf_TILED,
>>>>>>>>  	I915_FORMAT_MOD_Y_TILED,
>>>>>>>> @@ -1277,6 +1290,12 @@ static bool skl_mod_supported(uint32_t
>>>>>>> format, uint64_t modifier)
>>>>>>>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
>>>>>>>>  			return true;
>>>>>>>>  		/* fall through */
>>>>>>>> +	case DRM_FORMAT_NV12:
>>>>>>>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
>>>>>>>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
>>>>>>>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
>>>>>>>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
>>>>>>>> +			return true;
>>>>>>> On patch 5 and 6: this is a tad overkill, just put it below
>>>>>>> DRM_FORMAT_VYUY and let it fall through. It's not different from
>>>>>>> the other formats. :) Otherwise looks good, I'll wait until the
>>>>>>> changes from drm-misc- next are merged into drm-intel-next-queued
>>>>>>> then this series can be applied with the minor fixup.
>>>>>> Sure, thank you. I'll make the change. I did this to fix a review
>>>>>> comment - which said all modifiers aren’t Covered for NV12 - when
>>>>>> we
>>>>> earlier had it under VYUV. Would that be still okay?
>>>>> Seems only CCS isn't covered, so putting it there should be fine. :)
>>>> Oh okay - sure thank you. Will make the change and float the series.
>>> Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we need to
>>> add the other modifiers like MOD_LINEAR, MOD_X_TILED and
>> MOD_Y_TILED?
>>>> Regards
>>>> Vidya
>> This is a C switch, unless there is a break it will continue, so it works as
>> intended. :)
>>
>> For example format UYVY if modifier == Yf_TILED_CCS, it will ignore all the
>> lines with case and default, and continue to the end of the switch
>> statement, or until it hits a 'break' so the below:
> I am so sorry, I did not notice there was no break :( Got confused. Thank you.
Yeah, in general that's what the 'fall through' comment is for, to make it clear that the lack of break is intentional and not a bug. :)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Monday, May 7, 2018 1:59 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> Op 07-05-18 om 10:29 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Monday, May 7, 2018 1:55 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >> supported format for sprite plane
> >>
> >> Op 07-05-18 om 10:20 schreef Srinivas, Vidya:
> >>>> -----Original Message-----
> >>>> From: Srinivas, Vidya
> >>>> Sent: Monday, May 7, 2018 1:46 PM
> >>>> To: 'Maarten Lankhorst' <maarten.lankhorst@linux.intel.com>; intel-
> >>>> gfx@lists.freedesktop.org
> >>>> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>> supported format for sprite plane
> >>>>
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >>>>> Sent: Monday, May 7, 2018 1:44 PM
> >>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>>> gfx@lists.freedesktop.org
> >>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>>> supported format for sprite plane
> >>>>>
> >>>>> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
> >>>>>>> -----Original Message-----
> >>>>>>> From: Maarten Lankhorst
> >>>>>>> [mailto:maarten.lankhorst@linux.intel.com]
> >>>>>>> Sent: Monday, May 7, 2018 1:38 PM
> >>>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>>>>> gfx@lists.freedesktop.org
> >>>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>>>>> supported format for sprite plane
> >>>>>>>
> >>>>>>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> >>>>>>>> From: Chandra Konduru <chandra.konduru@intel.com>
> >>>>>>>>
> >>>>>>>> This patch adds NV12 to list of supported formats for sprite plane.
> >>>>>>>>
> >>>>>>>> v2: Rebased (me)
> >>>>>>>>
> >>>>>>>> v3: Review comments by Ville addressed
> >>>>>>>> - Removed skl_plane_formats_with_nv12 and added
> >>>>>>>> NV12 case in existing skl_plane_formats
> >>>>>>>> - Added the 10bpc RGB formats
> >>>>>>>>
> >>>>>>>> v4: Addressed review comments from Clinton A Taylor "Why are
> we
> >>>>>>> adding
> >>>>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>>>> Trying to set XR30 or AB30 results in error returned even
> >>>>>>>> though the modes are advertised for the planes"
> >>>>>>>> - Removed 10bit RGB formats added previously with NV12 series
> >>>>>>>>
> >>>>>>>> v5: Missed the Tested-by/Reviewed-by in the previous series
> >>>>>>>> Adding the same to commit message in this version.
> >>>>>>>> Addressed review comments from Clinton A Taylor "Why are we
> >>>> adding
> >>>>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>>>> Trying to set XR30 or AB30 results in error returned even
> >>>>>>>> though the modes are advertised for the planes"
> >>>>>>>> - Previous version has 10bit RGB format removed from VLV
> >>>>>>>> formats by mistake. Fixing that in this version.
> >>>>>>>> Removed 10bit RGB formats added previously with NV12 series
> for
> >>>> SKL.
> >>>>>>>> v6: Addressed review comments by Ville Restricting the NV12 to
> >>>>>>>> BXT and PIPE A and B
> >>>>>>>>
> >>>>>>>> v7: Rebased (me)
> >>>>>>>>
> >>>>>>>> v8: Rebased (me)
> >>>>>>>> Restricting NV12 changes to BXT and KBL Restricting NV12
> >>>>>>>> changes for plane 0 (overlay)
> >>>>>>>>
> >>>>>>>> v9: Rebased (me)
> >>>>>>>>
> >>>>>>>> v10: Addressed review comments from Maarten.
> >>>>>>>> Adding NV12 to skl_plane_formats itself.
> >>>>>>>>
> >>>>>>>> v11: Addressed review comments from Shashank Sharma
> >>>>>>>>
> >>>>>>>> v12: Addressed review comments from Shashank Sharma Made
> the
> >>>>>>> condition
> >>>>>>>> in intel_sprite_plane_create simple and easy to read as
> suggested.
> >>>>>>>>
> >>>>>>>> v13: Adding reviewed by tag from Shashank Sharma Addressed
> >> review
> >>>>>>>> comments from Juha-Pekka Heikkila
> >>>>>>>> "NV12 not to be supported by SKL"
> >>>>>>>>
> >>>>>>>> v14: Addressed review comments from Ville Added
> >>>> skl_planar_formats
> >>>>>>>> to include NV12 and a check skl_plane_has_planar in sprite
> >>>>>>>> create Added
> >>>>>>>> NV12 format to skl_mod_supported. These were review
> comments
> >>>>> from
> >>>>>>>> Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>>>>
> >>>>>>>> v15: Added reviewed by from Juha-Pekka Heikkila
> >>>>>>>>
> >>>>>>>> v16: Rebased the series
> >>>>>>>>
> >>>>>>>> v17: Added all tiling under mod supported for NV12 Credits to
> >>>>>>>> Megha Aggarwal
> >>>>>>>>
> >>>>>>>> v18: Added RB by Maarten and Kristian
> >>>>>>>>
> >>>>>>>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
> >>>>>>>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>>>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>>>> Reviewed-by: Maarten Lankhorst
> >>>> <maarten.lankhorst@linux.intel.com>
> >>>>>>>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>>>>>>> Reviewed-by: Juha-Pekka Heikkila
> <juhapekka.heikkila@gmail.com>
> >>>>>>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>>>>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>>>>>>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> >>>>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> >>>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>>>>>>> ---
> >>>>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 29
> >>>>>>>> +++++++++++++++++++++++++++--
> >>>>>>>>  1 file changed, 27 insertions(+), 2 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>> index c73553a..cdcae9e 100644
> >>>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] =
> {
> >>>>>>>>  	DRM_FORMAT_VYUY,
> >>>>>>>>  };
> >>>>>>>>
> >>>>>>>> +static uint32_t skl_planar_formats[] = {
> >>>>>>>> +	DRM_FORMAT_RGB565,
> >>>>>>>> +	DRM_FORMAT_ABGR8888,
> >>>>>>>> +	DRM_FORMAT_ARGB8888,
> >>>>>>>> +	DRM_FORMAT_XBGR8888,
> >>>>>>>> +	DRM_FORMAT_XRGB8888,
> >>>>>>>> +	DRM_FORMAT_YUYV,
> >>>>>>>> +	DRM_FORMAT_YVYU,
> >>>>>>>> +	DRM_FORMAT_UYVY,
> >>>>>>>> +	DRM_FORMAT_VYUY,
> >>>>>>>> +	DRM_FORMAT_NV12,
> >>>>>>>> +};
> >>>>>>>> +
> >>>>>>>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
> >>>>>>>>  	I915_FORMAT_MOD_Yf_TILED,
> >>>>>>>>  	I915_FORMAT_MOD_Y_TILED,
> >>>>>>>> @@ -1277,6 +1290,12 @@ static bool
> skl_mod_supported(uint32_t
> >>>>>>> format, uint64_t modifier)
> >>>>>>>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>>>>  			return true;
> >>>>>>>>  		/* fall through */
> >>>>>>>> +	case DRM_FORMAT_NV12:
> >>>>>>>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> >>>>>>>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
> >>>>>>>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
> >>>>>>>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>>>> +			return true;
> >>>>>>> On patch 5 and 6: this is a tad overkill, just put it below
> >>>>>>> DRM_FORMAT_VYUY and let it fall through. It's not different from
> >>>>>>> the other formats. :) Otherwise looks good, I'll wait until the
> >>>>>>> changes from drm-misc- next are merged into
> >>>>>>> drm-intel-next-queued then this series can be applied with the
> minor fixup.
> >>>>>> Sure, thank you. I'll make the change. I did this to fix a review
> >>>>>> comment - which said all modifiers aren’t Covered for NV12 - when
> >>>>>> we
> >>>>> earlier had it under VYUV. Would that be still okay?
> >>>>> Seems only CCS isn't covered, so putting it there should be fine.
> >>>>> :)
> >>>> Oh okay - sure thank you. Will make the change and float the series.
> >>> Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we need
> >>> to add the other modifiers like MOD_LINEAR, MOD_X_TILED and
> >> MOD_Y_TILED?
> >>>> Regards
> >>>> Vidya
> >> This is a C switch, unless there is a break it will continue, so it
> >> works as intended. :)
> >>
> >> For example format UYVY if modifier == Yf_TILED_CCS, it will ignore
> >> all the lines with case and default, and continue to the end of the
> >> switch statement, or until it hits a 'break' so the below:
> > I am so sorry, I did not notice there was no break :( Got confused. Thank
> you.
> Yeah, in general that's what the 'fall through' comment is for, to make it
> clear that the lack of break is intentional and not a bug. :)
Oh okay, thank you so much. I did not know about that. I have made the change
and refloated the series. Kindly have a check. https://patchwork.freedesktop.org/series/41674/
Rev 5.

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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Maarten Lankhorst]

Op 07-05-18 om 10:34 schreef Srinivas, Vidya:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>> Sent: Monday, May 7, 2018 1:59 PM
>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
>> format for sprite plane
>>
>> Op 07-05-18 om 10:29 schreef Srinivas, Vidya:
>>>> -----Original Message-----
>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>> Sent: Monday, May 7, 2018 1:55 PM
>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>> gfx@lists.freedesktop.org
>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>> supported format for sprite plane
>>>>
>>>> Op 07-05-18 om 10:20 schreef Srinivas, Vidya:
>>>>>> -----Original Message-----
>>>>>> From: Srinivas, Vidya
>>>>>> Sent: Monday, May 7, 2018 1:46 PM
>>>>>> To: 'Maarten Lankhorst' <maarten.lankhorst@linux.intel.com>; intel-
>>>>>> gfx@lists.freedesktop.org
>>>>>> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>>>> supported format for sprite plane
>>>>>>
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>>>>>>> Sent: Monday, May 7, 2018 1:44 PM
>>>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>>>>> gfx@lists.freedesktop.org
>>>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>>>>> supported format for sprite plane
>>>>>>>
>>>>>>> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Maarten Lankhorst
>>>>>>>>> [mailto:maarten.lankhorst@linux.intel.com]
>>>>>>>>> Sent: Monday, May 7, 2018 1:38 PM
>>>>>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
>>>>>>>>> gfx@lists.freedesktop.org
>>>>>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
>>>>>>>>> supported format for sprite plane
>>>>>>>>>
>>>>>>>>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
>>>>>>>>>> From: Chandra Konduru <chandra.konduru@intel.com>
>>>>>>>>>>
>>>>>>>>>> This patch adds NV12 to list of supported formats for sprite plane.
>>>>>>>>>>
>>>>>>>>>> v2: Rebased (me)
>>>>>>>>>>
>>>>>>>>>> v3: Review comments by Ville addressed
>>>>>>>>>> - Removed skl_plane_formats_with_nv12 and added
>>>>>>>>>> NV12 case in existing skl_plane_formats
>>>>>>>>>> - Added the 10bpc RGB formats
>>>>>>>>>>
>>>>>>>>>> v4: Addressed review comments from Clinton A Taylor "Why are
>> we
>>>>>>>>> adding
>>>>>>>>>> 10 bit RGB formats with the NV12 series patches?
>>>>>>>>>> Trying to set XR30 or AB30 results in error returned even
>>>>>>>>>> though the modes are advertised for the planes"
>>>>>>>>>> - Removed 10bit RGB formats added previously with NV12 series
>>>>>>>>>>
>>>>>>>>>> v5: Missed the Tested-by/Reviewed-by in the previous series
>>>>>>>>>> Adding the same to commit message in this version.
>>>>>>>>>> Addressed review comments from Clinton A Taylor "Why are we
>>>>>> adding
>>>>>>>>>> 10 bit RGB formats with the NV12 series patches?
>>>>>>>>>> Trying to set XR30 or AB30 results in error returned even
>>>>>>>>>> though the modes are advertised for the planes"
>>>>>>>>>> - Previous version has 10bit RGB format removed from VLV
>>>>>>>>>> formats by mistake. Fixing that in this version.
>>>>>>>>>> Removed 10bit RGB formats added previously with NV12 series
>> for
>>>>>> SKL.
>>>>>>>>>> v6: Addressed review comments by Ville Restricting the NV12 to
>>>>>>>>>> BXT and PIPE A and B
>>>>>>>>>>
>>>>>>>>>> v7: Rebased (me)
>>>>>>>>>>
>>>>>>>>>> v8: Rebased (me)
>>>>>>>>>> Restricting NV12 changes to BXT and KBL Restricting NV12
>>>>>>>>>> changes for plane 0 (overlay)
>>>>>>>>>>
>>>>>>>>>> v9: Rebased (me)
>>>>>>>>>>
>>>>>>>>>> v10: Addressed review comments from Maarten.
>>>>>>>>>> Adding NV12 to skl_plane_formats itself.
>>>>>>>>>>
>>>>>>>>>> v11: Addressed review comments from Shashank Sharma
>>>>>>>>>>
>>>>>>>>>> v12: Addressed review comments from Shashank Sharma Made
>> the
>>>>>>>>> condition
>>>>>>>>>> in intel_sprite_plane_create simple and easy to read as
>> suggested.
>>>>>>>>>> v13: Adding reviewed by tag from Shashank Sharma Addressed
>>>> review
>>>>>>>>>> comments from Juha-Pekka Heikkila
>>>>>>>>>> "NV12 not to be supported by SKL"
>>>>>>>>>>
>>>>>>>>>> v14: Addressed review comments from Ville Added
>>>>>> skl_planar_formats
>>>>>>>>>> to include NV12 and a check skl_plane_has_planar in sprite
>>>>>>>>>> create Added
>>>>>>>>>> NV12 format to skl_mod_supported. These were review
>> comments
>>>>>>> from
>>>>>>>>>> Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>>>>>>
>>>>>>>>>> v15: Added reviewed by from Juha-Pekka Heikkila
>>>>>>>>>>
>>>>>>>>>> v16: Rebased the series
>>>>>>>>>>
>>>>>>>>>> v17: Added all tiling under mod supported for NV12 Credits to
>>>>>>>>>> Megha Aggarwal
>>>>>>>>>>
>>>>>>>>>> v18: Added RB by Maarten and Kristian
>>>>>>>>>>
>>>>>>>>>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
>>>>>>>>>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>>>>>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>>>>>> Reviewed-by: Maarten Lankhorst
>>>>>> <maarten.lankhorst@linux.intel.com>
>>>>>>>>>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>>>>>>>>> Reviewed-by: Juha-Pekka Heikkila
>> <juhapekka.heikkila@gmail.com>
>>>>>>>>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
>>>>>>>>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
>>>>>>>>>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
>>>>>>>>>> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
>>>>>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>>>>>>>>> ---
>>>>>>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 29
>>>>>>>>>> +++++++++++++++++++++++++++--
>>>>>>>>>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>>>> index c73553a..cdcae9e 100644
>>>>>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>>>>>>>>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[] =
>> {
>>>>>>>>>>  	DRM_FORMAT_VYUY,
>>>>>>>>>>  };
>>>>>>>>>>
>>>>>>>>>> +static uint32_t skl_planar_formats[] = {
>>>>>>>>>> +	DRM_FORMAT_RGB565,
>>>>>>>>>> +	DRM_FORMAT_ABGR8888,
>>>>>>>>>> +	DRM_FORMAT_ARGB8888,
>>>>>>>>>> +	DRM_FORMAT_XBGR8888,
>>>>>>>>>> +	DRM_FORMAT_XRGB8888,
>>>>>>>>>> +	DRM_FORMAT_YUYV,
>>>>>>>>>> +	DRM_FORMAT_YVYU,
>>>>>>>>>> +	DRM_FORMAT_UYVY,
>>>>>>>>>> +	DRM_FORMAT_VYUY,
>>>>>>>>>> +	DRM_FORMAT_NV12,
>>>>>>>>>> +};
>>>>>>>>>> +
>>>>>>>>>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
>>>>>>>>>>  	I915_FORMAT_MOD_Yf_TILED,
>>>>>>>>>>  	I915_FORMAT_MOD_Y_TILED,
>>>>>>>>>> @@ -1277,6 +1290,12 @@ static bool
>> skl_mod_supported(uint32_t
>>>>>>>>> format, uint64_t modifier)
>>>>>>>>>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
>>>>>>>>>>  			return true;
>>>>>>>>>>  		/* fall through */
>>>>>>>>>> +	case DRM_FORMAT_NV12:
>>>>>>>>>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
>>>>>>>>>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
>>>>>>>>>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
>>>>>>>>>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
>>>>>>>>>> +			return true;
>>>>>>>>> On patch 5 and 6: this is a tad overkill, just put it below
>>>>>>>>> DRM_FORMAT_VYUY and let it fall through. It's not different from
>>>>>>>>> the other formats. :) Otherwise looks good, I'll wait until the
>>>>>>>>> changes from drm-misc- next are merged into
>>>>>>>>> drm-intel-next-queued then this series can be applied with the
>> minor fixup.
>>>>>>>> Sure, thank you. I'll make the change. I did this to fix a review
>>>>>>>> comment - which said all modifiers aren’t Covered for NV12 - when
>>>>>>>> we
>>>>>>> earlier had it under VYUV. Would that be still okay?
>>>>>>> Seems only CCS isn't covered, so putting it there should be fine.
>>>>>>> :)
>>>>>> Oh okay - sure thank you. Will make the change and float the series.
>>>>> Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we need
>>>>> to add the other modifiers like MOD_LINEAR, MOD_X_TILED and
>>>> MOD_Y_TILED?
>>>>>> Regards
>>>>>> Vidya
>>>> This is a C switch, unless there is a break it will continue, so it
>>>> works as intended. :)
>>>>
>>>> For example format UYVY if modifier == Yf_TILED_CCS, it will ignore
>>>> all the lines with case and default, and continue to the end of the
>>>> switch statement, or until it hits a 'break' so the below:
>>> I am so sorry, I did not notice there was no break :( Got confused. Thank
>> you.
>> Yeah, in general that's what the 'fall through' comment is for, to make it
>> clear that the lack of break is intentional and not a bug. :)
> Oh okay, thank you so much. I did not know about that. I have made the change
> and refloated the series. Kindly have a check. https://patchwork.freedesktop.org/series/41674/
> Rev 5.
>
> Regards
> Vidya

Ack, looking good.

Now we only have to wait for the drm-misc-next backmerge, then I will commit this.

~Maarten

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

Re: [PATCH v6 6/6] drm/i915: Add NV12 as supported format for sprite plane

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Monday, May 7, 2018 2:08 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as supported
> format for sprite plane
> 
> Op 07-05-18 om 10:34 schreef Srinivas, Vidya:
> >
> >> -----Original Message-----
> >> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >> Sent: Monday, May 7, 2018 1:59 PM
> >> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >> gfx@lists.freedesktop.org
> >> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >> supported format for sprite plane
> >>
> >> Op 07-05-18 om 10:29 schreef Srinivas, Vidya:
> >>>> -----Original Message-----
> >>>> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> >>>> Sent: Monday, May 7, 2018 1:55 PM
> >>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>> gfx@lists.freedesktop.org
> >>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>> supported format for sprite plane
> >>>>
> >>>> Op 07-05-18 om 10:20 schreef Srinivas, Vidya:
> >>>>>> -----Original Message-----
> >>>>>> From: Srinivas, Vidya
> >>>>>> Sent: Monday, May 7, 2018 1:46 PM
> >>>>>> To: 'Maarten Lankhorst' <maarten.lankhorst@linux.intel.com>;
> >>>>>> intel- gfx@lists.freedesktop.org
> >>>>>> Subject: RE: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>>>> supported format for sprite plane
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Maarten Lankhorst
> >>>>>>> [mailto:maarten.lankhorst@linux.intel.com]
> >>>>>>> Sent: Monday, May 7, 2018 1:44 PM
> >>>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>>>>> gfx@lists.freedesktop.org
> >>>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>>>>> supported format for sprite plane
> >>>>>>>
> >>>>>>> Op 07-05-18 om 10:11 schreef Srinivas, Vidya:
> >>>>>>>>> -----Original Message-----
> >>>>>>>>> From: Maarten Lankhorst
> >>>>>>>>> [mailto:maarten.lankhorst@linux.intel.com]
> >>>>>>>>> Sent: Monday, May 7, 2018 1:38 PM
> >>>>>>>>> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> >>>>>>>>> gfx@lists.freedesktop.org
> >>>>>>>>> Subject: Re: [Intel-gfx] [PATCH v6 6/6] drm/i915: Add NV12 as
> >>>>>>>>> supported format for sprite plane
> >>>>>>>>>
> >>>>>>>>> Op 06-05-18 om 19:44 schreef Vidya Srinivas:
> >>>>>>>>>> From: Chandra Konduru <chandra.konduru@intel.com>
> >>>>>>>>>>
> >>>>>>>>>> This patch adds NV12 to list of supported formats for sprite
> plane.
> >>>>>>>>>>
> >>>>>>>>>> v2: Rebased (me)
> >>>>>>>>>>
> >>>>>>>>>> v3: Review comments by Ville addressed
> >>>>>>>>>> - Removed skl_plane_formats_with_nv12 and added
> >>>>>>>>>> NV12 case in existing skl_plane_formats
> >>>>>>>>>> - Added the 10bpc RGB formats
> >>>>>>>>>>
> >>>>>>>>>> v4: Addressed review comments from Clinton A Taylor "Why
> are
> >> we
> >>>>>>>>> adding
> >>>>>>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>>>>>> Trying to set XR30 or AB30 results in error returned even
> >>>>>>>>>> though the modes are advertised for the planes"
> >>>>>>>>>> - Removed 10bit RGB formats added previously with NV12
> series
> >>>>>>>>>>
> >>>>>>>>>> v5: Missed the Tested-by/Reviewed-by in the previous series
> >>>>>>>>>> Adding the same to commit message in this version.
> >>>>>>>>>> Addressed review comments from Clinton A Taylor "Why are
> we
> >>>>>> adding
> >>>>>>>>>> 10 bit RGB formats with the NV12 series patches?
> >>>>>>>>>> Trying to set XR30 or AB30 results in error returned even
> >>>>>>>>>> though the modes are advertised for the planes"
> >>>>>>>>>> - Previous version has 10bit RGB format removed from VLV
> >>>>>>>>>> formats by mistake. Fixing that in this version.
> >>>>>>>>>> Removed 10bit RGB formats added previously with NV12 series
> >> for
> >>>>>> SKL.
> >>>>>>>>>> v6: Addressed review comments by Ville Restricting the NV12
> >>>>>>>>>> to BXT and PIPE A and B
> >>>>>>>>>>
> >>>>>>>>>> v7: Rebased (me)
> >>>>>>>>>>
> >>>>>>>>>> v8: Rebased (me)
> >>>>>>>>>> Restricting NV12 changes to BXT and KBL Restricting NV12
> >>>>>>>>>> changes for plane 0 (overlay)
> >>>>>>>>>>
> >>>>>>>>>> v9: Rebased (me)
> >>>>>>>>>>
> >>>>>>>>>> v10: Addressed review comments from Maarten.
> >>>>>>>>>> Adding NV12 to skl_plane_formats itself.
> >>>>>>>>>>
> >>>>>>>>>> v11: Addressed review comments from Shashank Sharma
> >>>>>>>>>>
> >>>>>>>>>> v12: Addressed review comments from Shashank Sharma Made
> >> the
> >>>>>>>>> condition
> >>>>>>>>>> in intel_sprite_plane_create simple and easy to read as
> >> suggested.
> >>>>>>>>>> v13: Adding reviewed by tag from Shashank Sharma Addressed
> >>>> review
> >>>>>>>>>> comments from Juha-Pekka Heikkila
> >>>>>>>>>> "NV12 not to be supported by SKL"
> >>>>>>>>>>
> >>>>>>>>>> v14: Addressed review comments from Ville Added
> >>>>>> skl_planar_formats
> >>>>>>>>>> to include NV12 and a check skl_plane_has_planar in sprite
> >>>>>>>>>> create Added
> >>>>>>>>>> NV12 format to skl_mod_supported. These were review
> >> comments
> >>>>>>> from
> >>>>>>>>>> Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>>>>>>
> >>>>>>>>>> v15: Added reviewed by from Juha-Pekka Heikkila
> >>>>>>>>>>
> >>>>>>>>>> v16: Rebased the series
> >>>>>>>>>>
> >>>>>>>>>> v17: Added all tiling under mod supported for NV12 Credits to
> >>>>>>>>>> Megha Aggarwal
> >>>>>>>>>>
> >>>>>>>>>> v18: Added RB by Maarten and Kristian
> >>>>>>>>>>
> >>>>>>>>>> Credits-to: Megha Aggarwal <megha.aggarwal@intel.com>
> >>>>>>>>>> Credits-to: Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>>>>>> Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
> >>>>>>>>>> Reviewed-by: Maarten Lankhorst
> >>>>>> <maarten.lankhorst@linux.intel.com>
> >>>>>>>>>> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>>>>>>>>> Reviewed-by: Juha-Pekka Heikkila
> >> <juhapekka.heikkila@gmail.com>
> >>>>>>>>>> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
> >>>>>>>>>> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> >>>>>>>>>> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> >>>>>>>>>> Signed-off-by: Nabendu Maiti
> <nabendu.bikash.maiti@intel.com>
> >>>>>>>>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> >>>>>>>>>> ---
> >>>>>>>>>>  drivers/gpu/drm/i915/intel_sprite.c | 29
> >>>>>>>>>> +++++++++++++++++++++++++++--
> >>>>>>>>>>  1 file changed, 27 insertions(+), 2 deletions(-)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>>>> b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>>>> index c73553a..cdcae9e 100644
> >>>>>>>>>> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>>>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >>>>>>>>>> @@ -1180,6 +1180,19 @@ static uint32_t skl_plane_formats[]
> =
> >> {
> >>>>>>>>>>  	DRM_FORMAT_VYUY,
> >>>>>>>>>>  };
> >>>>>>>>>>
> >>>>>>>>>> +static uint32_t skl_planar_formats[] = {
> >>>>>>>>>> +	DRM_FORMAT_RGB565,
> >>>>>>>>>> +	DRM_FORMAT_ABGR8888,
> >>>>>>>>>> +	DRM_FORMAT_ARGB8888,
> >>>>>>>>>> +	DRM_FORMAT_XBGR8888,
> >>>>>>>>>> +	DRM_FORMAT_XRGB8888,
> >>>>>>>>>> +	DRM_FORMAT_YUYV,
> >>>>>>>>>> +	DRM_FORMAT_YVYU,
> >>>>>>>>>> +	DRM_FORMAT_UYVY,
> >>>>>>>>>> +	DRM_FORMAT_VYUY,
> >>>>>>>>>> +	DRM_FORMAT_NV12,
> >>>>>>>>>> +};
> >>>>>>>>>> +
> >>>>>>>>>>  static const uint64_t skl_plane_format_modifiers_noccs[] = {
> >>>>>>>>>>  	I915_FORMAT_MOD_Yf_TILED,
> >>>>>>>>>>  	I915_FORMAT_MOD_Y_TILED,
> >>>>>>>>>> @@ -1277,6 +1290,12 @@ static bool
> >> skl_mod_supported(uint32_t
> >>>>>>>>> format, uint64_t modifier)
> >>>>>>>>>>  		if (modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>>>>>>  			return true;
> >>>>>>>>>>  		/* fall through */
> >>>>>>>>>> +	case DRM_FORMAT_NV12:
> >>>>>>>>>> +		if (modifier == DRM_FORMAT_MOD_LINEAR ||
> >>>>>>>>>> +		    modifier == I915_FORMAT_MOD_X_TILED ||
> >>>>>>>>>> +		    modifier == I915_FORMAT_MOD_Y_TILED ||
> >>>>>>>>>> +		    modifier == I915_FORMAT_MOD_Yf_TILED)
> >>>>>>>>>> +			return true;
> >>>>>>>>> On patch 5 and 6: this is a tad overkill, just put it below
> >>>>>>>>> DRM_FORMAT_VYUY and let it fall through. It's not different
> >>>>>>>>> from the other formats. :) Otherwise looks good, I'll wait
> >>>>>>>>> until the changes from drm-misc- next are merged into
> >>>>>>>>> drm-intel-next-queued then this series can be applied with the
> >> minor fixup.
> >>>>>>>> Sure, thank you. I'll make the change. I did this to fix a
> >>>>>>>> review comment - which said all modifiers aren’t Covered for
> >>>>>>>> NV12 - when we
> >>>>>>> earlier had it under VYUV. Would that be still okay?
> >>>>>>> Seems only CCS isn't covered, so putting it there should be fine.
> >>>>>>> :)
> >>>>>> Oh okay - sure thank you. Will make the change and float the series.
> >>>>> Sorry to ask - under VYUV only Yf_TILED returns true. Don’t we
> >>>>> need to add the other modifiers like MOD_LINEAR, MOD_X_TILED and
> >>>> MOD_Y_TILED?
> >>>>>> Regards
> >>>>>> Vidya
> >>>> This is a C switch, unless there is a break it will continue, so it
> >>>> works as intended. :)
> >>>>
> >>>> For example format UYVY if modifier == Yf_TILED_CCS, it will ignore
> >>>> all the lines with case and default, and continue to the end of the
> >>>> switch statement, or until it hits a 'break' so the below:
> >>> I am so sorry, I did not notice there was no break :( Got confused.
> >>> Thank
> >> you.
> >> Yeah, in general that's what the 'fall through' comment is for, to
> >> make it clear that the lack of break is intentional and not a bug. :)
> > Oh okay, thank you so much. I did not know about that. I have made the
> > change and refloated the series. Kindly have a check.
> > https://patchwork.freedesktop.org/series/41674/
> > Rev 5.
> >
> > Regards
> > Vidya
> 
> Ack, looking good.
> 
> Now we only have to wait for the drm-misc-next backmerge, then I will
> commit this.

:) Sure thank you :)

> 
> ~Maarten

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

[PATCH v6 4/6] drm/i915: Add NV12 support to intel_framebuffer_init

[Vidya Srinivas]

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

This patch adds NV12 as supported format
to intel_framebuffer_init and performs various checks.

v2:
-Fix an issue in checks added (Chandra Konduru)

v3: rebased (me)

v4: Review comments by Ville addressed
Added platform check for NV12 in intel_framebuffer_init
Removed offset checks for NV12 case

v5: Addressed review comments by Clinton A Taylor
This NV12 support only correctly works on SKL.
Plane color space conversion is different on GLK and later platforms
causing the colors to display incorrectly.
Ville's plane color space property patch series
in review will fix this issue.
- Restricted the NV12 case in intel_framebuffer_init to
SKL and BXT only.

v6: Rebased (me)

v7: Addressed review comments by Ville
Restricting the NV12 to BXT for now.

v8: Rebased (me)
Restricting the NV12 changes to BXT and KBL for now.

v9: Rebased (me)

v10: NV12 supported by all GEN >= 9.
Making this change in intel_framebuffer_init. This is
part of addressing Maarten's review comments.
Comment under v8 no longer applicable

v11: Addressed review comments from Shashank Sharma

v12: Adding Reviewed By from Shashank Sharma

v13: Addressed review comments from Juha-Pekka Heikkila
"NV12 not to be supported by SKL"

v14: Addressed review comments from Maarten.
Add checks for fb width height for NV12 and fail the fb
creation if check fails. Added reviewed by from
Juha-Pekka Heikkila

v15: Rebased the series

v16: Setting the minimum value during fb creating to 16
as per Bspec for NV12. Earlier minimum was expected
to be > 16. Now changed it to >=16.

v17: Adding restriction to framebuffer_init - the fb
width and height should be a multiplier of 4

v18: Added RB from Maarten. Included Maarten's review comments
Dont allow CCS formats for fb creation of NV12

v19: Review comments from Maarten addressed -
Removing BROXTON support for NV12 due to WA826

Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f1263e9..335bc71 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14304,6 +14304,20 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 			goto err;
 		}
 		break;
+	case DRM_FORMAT_NV12:
+		if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED_CCS ||
+		    mode_cmd->modifier[0] == I915_FORMAT_MOD_Yf_TILED_CCS) {
+			DRM_DEBUG_KMS("RC not to be enabled with NV12\n");
+			goto err;
+		}
+		if (INTEL_GEN(dev_priv) < 9 || IS_SKYLAKE(dev_priv) ||
+		    IS_BROXTON(dev_priv)) {
+			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));
@@ -14316,6 +14330,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 
 	drm_helper_mode_fill_fb_struct(&dev_priv->drm, fb, mode_cmd);
 
+	if (fb->format->format == DRM_FORMAT_NV12 &&
+	    (fb->width < SKL_MIN_YUV_420_SRC_W ||
+	     fb->height < SKL_MIN_YUV_420_SRC_H ||
+	     (fb->width % 4) != 0 || (fb->height % 4) != 0)) {
+		DRM_DEBUG_KMS("src dimensions not correct for NV12\n");
+		return -EINVAL;
+	}
+
 	for (i = 0; i < fb->format->num_planes; i++) {
 		u32 stride_alignment;
 
-- 
2.7.4

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

[PATCH v6 5/6] drm/i915: Add NV12 as supported format for primary plane

[Vidya Srinivas]

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

This patch adds NV12 to list of supported formats for
primary plane

v2: Rebased (Chandra Konduru)

v3: Rebased (me)

v4: Review comments by Ville addressed
Removed the skl_primary_formats_with_nv12 and
added NV12 case in existing skl_primary_formats

v5: Rebased (me)

v6: Missed the Tested-by/Reviewed-by in the previous series
Adding the same to commit message in this version.

v7: Review comments by Ville addressed
	Restricting the NV12 for BXT and on PIPE A and B
Rebased (me)

v8: Rebased (me)
Modified restricting the NV12 support for both BXT and KBL.

v9: Rebased (me)

v10: Addressed review comments from Maarten.
	Adding NV12 inside skl_primary_formats itself.

v11: Adding Reviewed By tag from Shashank Sharma

v12: Addressed review comments from Juha-Pekka Heikkila
"NV12 not to be supported by SKL"

v13: Addressed review comments from Ville
Added skl_pri_planar_formats to include NV12
and skl_plane_has_planar function to check for
NV12 support on plane. Added NV12 format to
skl_mod_supported. These were review comments
from Kristian Høgsberg <hoegsberg@gmail.com>

v14: Added reviewed by from Juha-Pekka Heikkila

v15: Rebased the series

v16: Added all tiling support under mod supported
for NV12. Credits to Megha Aggarwal

v17: Added RB by Maarten and Kristian

v18: Review comments from Maarten addressed -
Removing BROXTON support for NV12 due to WA826

Credits-to: Megha Aggarwal megha.aggarwal@intel.com
Credits-to: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Kristian Høgsberg <hoegsberg@gmail.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 55 ++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 335bc71..bf7f174 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -88,6 +88,22 @@ static const uint32_t skl_primary_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
+static const uint32_t skl_pri_planar_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,
+	DRM_FORMAT_NV12,
+};
+
 static const uint64_t skl_format_modifiers_noccs[] = {
 	I915_FORMAT_MOD_Yf_TILED,
 	I915_FORMAT_MOD_Y_TILED,
@@ -13218,6 +13234,12 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier)
 		if (modifier == I915_FORMAT_MOD_Yf_TILED)
 			return true;
 		/* fall through */
+	case DRM_FORMAT_NV12:
+		if (modifier == DRM_FORMAT_MOD_LINEAR ||
+		    modifier == I915_FORMAT_MOD_X_TILED ||
+		    modifier == I915_FORMAT_MOD_Y_TILED ||
+		    modifier == I915_FORMAT_MOD_Yf_TILED)
+			return true;
 	case DRM_FORMAT_C8:
 		if (modifier == DRM_FORMAT_MOD_LINEAR ||
 		    modifier == I915_FORMAT_MOD_X_TILED ||
@@ -13422,6 +13444,30 @@ static bool skl_plane_has_fbc(struct drm_i915_private *dev_priv,
 	return pipe == PIPE_A && plane_id == PLANE_PRIMARY;
 }
 
+bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
+			  enum pipe pipe, enum plane_id plane_id)
+{
+	if (plane_id == PLANE_PRIMARY) {
+		if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
+			return false;
+		else if ((INTEL_GEN(dev_priv) == 9 && pipe == PIPE_C) &&
+			 !IS_GEMINILAKE(dev_priv))
+			return false;
+	} else if (plane_id >= PLANE_SPRITE0) {
+		if (plane_id == PLANE_CURSOR)
+			return false;
+		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) == 10) {
+			if (plane_id != PLANE_SPRITE0)
+				return false;
+		} else {
+			if (plane_id != PLANE_SPRITE0 || pipe == PIPE_C ||
+			    IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv))
+				return false;
+		}
+	}
+	return true;
+}
+
 static struct intel_plane *
 intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
@@ -13482,8 +13528,13 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
 	primary->check_plane = intel_check_primary_plane;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
-		intel_primary_formats = skl_primary_formats;
-		num_formats = ARRAY_SIZE(skl_primary_formats);
+		if (skl_plane_has_planar(dev_priv, pipe, PLANE_PRIMARY)) {
+			intel_primary_formats = skl_pri_planar_formats;
+			num_formats = ARRAY_SIZE(skl_pri_planar_formats);
+		} else {
+			intel_primary_formats = skl_primary_formats;
+			num_formats = ARRAY_SIZE(skl_primary_formats);
+		}
 
 		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
 			modifiers = skl_format_modifiers_ccs;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9a13eeb..8049883 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2076,6 +2076,8 @@ bool skl_plane_get_hw_state(struct intel_plane *plane);
 bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
 		       enum pipe pipe, enum plane_id plane_id);
 bool intel_format_is_yuv(uint32_t format);
+bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
+			  enum pipe pipe, enum plane_id plane_id);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_i915_private *dev_priv);
-- 
2.7.4

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

✗ Fi.CI.CHECKPATCH: warning for Enable NV12 support (rev4)

[Patchwork]

== Series Details ==

Series: Enable NV12 support (rev4)
URL   : https://patchwork.freedesktop.org/series/41674/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
fbc1497e23e6 drm/i915: Enable display workaround 827 for all planes, v2.
-:64: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#64: FILE: drivers/gpu/drm/i915/intel_display.c:5155:
+	if ((INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv)) ||
+	       IS_CANNONLAKE(dev_priv))

total: 0 errors, 0 warnings, 1 checks, 98 lines checked
2ed474bbd47e drm/i915: Enable Display WA 0528
04a857fa4763 drm/i915: Add skl_check_nv12_surface for NV12
b4d7e9889b5b drm/i915: Add NV12 support to intel_framebuffer_init
b25b811c7220 drm/i915: Add NV12 as supported format for primary plane
7e66aea5ce91 drm/i915: Add NV12 as supported format for sprite plane

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

✓ Fi.CI.BAT: success for Enable NV12 support (rev4)

[Patchwork]

== Series Details ==

Series: Enable NV12 support (rev4)
URL   : https://patchwork.freedesktop.org/series/41674/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4149 -> Patchwork_8915 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41674/revisions/4/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-4200u:       PASS -> DMESG-FAIL (fdo#102614)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cfl-s3:          PASS -> FAIL (fdo#103375) +1

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@gem_exec_suspend@basic-s4-devices:
      fi-skl-guc:         FAIL (fdo#105900, fdo#104699) -> PASS +1

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cnl-psr:         FAIL (fdo#100368) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      {fi-hsw-peppy}:     DMESG-FAIL (fdo#106103, fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

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

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104699 https://bugs.freedesktop.org/show_bug.cgi?id=104699
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103


== Participating hosts (40 -> 36) ==

  Missing    (4): fi-byt-j1900 fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4149 -> Patchwork_8915

  CI_DRM_4149: 6c2ec0dee7d19b798a1de1101175f5a076549cd9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4461: f772d9a910130b3aec8efa4f09ed723618fae656 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8915: 7e66aea5ce91de80f1f7c8e52d15075a9f9bf4e6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4461: 55207ea5154dfaa6d2c128124c50e3be4f9b6440 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

7e66aea5ce91 drm/i915: Add NV12 as supported format for sprite plane
b25b811c7220 drm/i915: Add NV12 as supported format for primary plane
b4d7e9889b5b drm/i915: Add NV12 support to intel_framebuffer_init
04a857fa4763 drm/i915: Add skl_check_nv12_surface for NV12
2ed474bbd47e drm/i915: Enable Display WA 0528
fbc1497e23e6 drm/i915: Enable display workaround 827 for all planes, v2.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8915/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for Enable NV12 support (rev4)

[Patchwork]

== Series Details ==

Series: Enable NV12 support (rev4)
URL   : https://patchwork.freedesktop.org/series/41674/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4149_full -> Patchwork_8915_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8915_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8915_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/41674/revisions/4/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_8915_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-bsd1:
      shard-kbl:          SKIP -> PASS

    igt@gem_mocs_settings@mocs-rc6-ctx-render:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@flip-vs-wf_vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105312)

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_rotation_crc@sprite-rotation-270:
      shard-apl:          PASS -> FAIL (fdo#103925)

    igt@kms_sysfs_edid_timing:
      shard-apl:          PASS -> WARN (fdo#100047)

    igt@kms_vblank@pipe-c-accuracy-idle:
      shard-hsw:          PASS -> FAIL (fdo#102583)

    igt@kms_vblank@pipe-c-ts-continuation-suspend:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665) +1

    
    ==== Possible fixes ====

    igt@kms_flip@absolute-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#106087) -> PASS

    igt@kms_setmode@basic:
      shard-glk:          FAIL (fdo#99912) -> PASS

    igt@prime_vgem@basic-fence-flip:
      shard-kbl:          DMESG-WARN (fdo#106247) -> PASS

    
  fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105312 https://bugs.freedesktop.org/show_bug.cgi?id=105312
  fdo#106087 https://bugs.freedesktop.org/show_bug.cgi?id=106087
  fdo#106247 https://bugs.freedesktop.org/show_bug.cgi?id=106247
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4149 -> Patchwork_8915

  CI_DRM_4149: 6c2ec0dee7d19b798a1de1101175f5a076549cd9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4461: f772d9a910130b3aec8efa4f09ed723618fae656 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8915: 7e66aea5ce91de80f1f7c8e52d15075a9f9bf4e6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4461: 55207ea5154dfaa6d2c128124c50e3be4f9b6440 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8915/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx