[PATCH 0/6] Adding NV12 support

[PATCH 0/6] Adding NV12 support

[Vidya Srinivas]

This patch series is adding NV12 support for Broxton display after
rebasing on latest drm-intel-nightly. Initial series of the patches
can be found here:
https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.html

Previous revision history:
Patches were initial reviewed last when floated but
currently there was a design change with respect to
- the way fb offset is handled
- the way rotation is handled
Rebase of the current NV12 patch series has been done as per the
current changes on drm-intel-nightly.
Review comments from Ville (12th June 2017) have been addressed
Review comments from Clinton A Taylor (7th July 2017) have been
addressed
Review comments from Clinton A Taylor (10th July 2017) have been
addressed. Had missed out tested-by/reviewed-by in the patches.
Fixed that error in this series.
Review comments from Ville (11th July 2017) addressed.
Review comments from Paauwe, Bob (29th July 2017) addressed.

Update from last rev:
Rebased the series as Ville's patches are merged. Previously,
this series included those floating patches.

Chandra Konduru (6):
  drm/i915: Set scaler mode for NV12
  drm/i915: Update format_is_yuv() to include NV12
  drm/i915: Upscale scaler max scale for NV12
  drm/i915: Add NV12 as supported format for primary plane
  drm/i915: Add NV12 as supported format for sprite plane
  drm/i915: Add NV12 support to intel_framebuffer_init

 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_atomic.c  |  8 ++++-
 drivers/gpu/drm/i915/intel_display.c | 67 +++++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 drivers/gpu/drm/i915/intel_sprite.c  | 34 ++++++++++++++----
 5 files changed, 92 insertions(+), 21 deletions(-)

-- 
1.9.1

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

[PATCH 1/6] drm/i915: Set scaler mode for NV12

[Vidya Srinivas]

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

This patch sets appropriate scaler mode for NV12 format.
In this mode, skylake scaler does either chroma-upsampling or
chroma-upsampling and resolution scaling

v2: Review comments from Ville addressed
	NV12 case to be checked first for setting
	the scaler

v3: Rebased (me)

v4: Rebased (me)

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

v6: Rebased (me)

v7: Rebased (me)

Tested-by: Clinton Taylor <clinton.a.taylor@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/i915_reg.h     | 1 +
 drivers/gpu/drm/i915/intel_atomic.c | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e2908ae..ac61135 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6582,6 +6582,7 @@ enum {
 #define PS_SCALER_MODE_MASK (3 << 28)
 #define PS_SCALER_MODE_DYN  (0 << 28)
 #define PS_SCALER_MODE_HQ  (1 << 28)
+#define PS_SCALER_MODE_NV12 (2 << 28)
 #define PS_PLANE_SEL_MASK  (7 << 25)
 #define PS_PLANE_SEL(plane) (((plane) + 1) << 25)
 #define PS_FILTER_MASK         (3 << 23)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 36d4e63..808f8e6 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -325,7 +325,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
 		}
 
 		/* set scaler mode */
-		if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
+		if (plane_state && plane_state->base.fb &&
+			plane_state->base.fb->format->format ==
+			DRM_FORMAT_NV12) {
+			DRM_ERROR("NV12 format setting scaler mode\n");
+			scaler_state->scalers[*scaler_id].mode =
+				PS_SCALER_MODE_NV12;
+		} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
 			scaler_state->scalers[*scaler_id].mode = 0;
 		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
 			/*
-- 
1.9.1

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

[PATCH 2/6] drm/i915: Update format_is_yuv() to include NV12

[Vidya Srinivas]

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

This patch adds NV12 to format_is_yuv() function
for sprite planes.

v2:
-Use intel_ prefix for format_is_yuv (Ville)

v3: Rebased (me)

v4: Rebased and addressed review comments from Clinton A Taylor.
	"static function in intel_sprite.c is not available
	to the primary plane functions".
	Changed commit message - function modified for
	sprite planes.

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

v6: Rebased (me)

v7: Rebased (me)

Tested-by: Clinton Taylor <clinton.a.taylor@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 | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 524933b..54f876e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -42,13 +42,14 @@
 #include "i915_drv.h"
 
 static bool
-format_is_yuv(uint32_t format)
+intel_format_is_yuv(uint32_t format)
 {
 	switch (format) {
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
 	case DRM_FORMAT_YVYU:
+	case DRM_FORMAT_NV12:
 		return true;
 	default:
 		return false;
@@ -331,7 +332,7 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
 	enum plane_id plane_id = plane->id;
 
 	/* Seems RGB data bypasses the CSC always */
-	if (!format_is_yuv(format))
+	if (!intel_format_is_yuv(format))
 		return;
 
 	/*
@@ -895,7 +896,7 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
 		src_y = src->y1 >> 16;
 		src_h = drm_rect_height(src) >> 16;
 
-		if (format_is_yuv(fb->format->format)) {
+		if (intel_format_is_yuv(fb->format->format)) {
 			src_x &= ~1;
 			src_w &= ~1;
 
-- 
1.9.1

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

[PATCH 3/6] drm/i915: Upscale scaler max scale for NV12

[Vidya Srinivas]

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

This patch updates scaler max limit support for NV12

v2: Rebased (me)

v3: Rebased (me)

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

v5: Addressed review comments from Ville and rebased
	- calculation of max_scale to be made
	less convoluted by splitting it up a bit
	- Indentation errors to be fixed in the series

v6: Rebased (me)
	Fixed review comments from Paauwe, Bob J
	Previous version, where a split of calculation
	was done, was wrong. Fixed that issue here.

v7: Rebased (me)

Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 33 +++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 drivers/gpu/drm/i915/intel_sprite.c  |  3 ++-
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7cd392f..4e73d88 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3451,6 +3451,8 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
 		return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_UYVY;
 	case DRM_FORMAT_VYUY:
 		return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
+	case DRM_FORMAT_NV12:
+		return PLANE_CTL_FORMAT_NV12;
 	default:
 		MISSING_CASE(pixel_format);
 	}
@@ -4707,7 +4709,8 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		  unsigned int scaler_user, int *scaler_id,
-		  int src_w, int src_h, int dst_w, int dst_h)
+		  int src_w, int src_h, int dst_w, int dst_h,
+		  uint32_t pixel_format)
 {
 	struct intel_crtc_scaler_state *scaler_state =
 		&crtc_state->scaler_state;
@@ -4723,7 +4726,8 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 	 * the 90/270 degree plane rotation cases (to match the
 	 * GTT mapping), hence no need to account for rotation here.
 	 */
-	need_scaling = src_w != dst_w || src_h != dst_h;
+	need_scaling = src_w != dst_w || src_h != dst_h ||
+		(pixel_format == DRM_FORMAT_NV12);
 
 	if (crtc_state->ycbcr420 && scaler_user == SKL_CRTC_INDEX)
 		need_scaling = true;
@@ -4802,7 +4806,7 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
 	return skl_update_scaler(state, !state->base.active, SKL_CRTC_INDEX,
 		&state->scaler_state.scaler_id,
 		state->pipe_src_w, state->pipe_src_h,
-		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
+		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay, 0);
 }
 
 /**
@@ -4832,7 +4836,8 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 				drm_rect_width(&plane_state->base.src) >> 16,
 				drm_rect_height(&plane_state->base.src) >> 16,
 				drm_rect_width(&plane_state->base.dst),
-				drm_rect_height(&plane_state->base.dst));
+				drm_rect_height(&plane_state->base.dst),
+				fb ? fb->format->format : 0);
 
 	if (ret || plane_state->scaler_id < 0)
 		return ret;
@@ -4858,6 +4863,7 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_YVYU:
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
+	case DRM_FORMAT_NV12:
 		break;
 	default:
 		DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
@@ -12836,11 +12842,12 @@ static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
 }
 
 int
-skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
+skl_max_scale(struct intel_crtc *intel_crtc,
+	struct intel_crtc_state *crtc_state, uint32_t pixel_format)
 {
 	struct drm_i915_private *dev_priv;
-	int max_scale;
-	int crtc_clock, max_dotclk;
+	int max_scale, mult;
+	int crtc_clock, max_dotclk, tmpclk1, tmpclk2;
 
 	if (!intel_crtc || !crtc_state->base.enable)
 		return DRM_PLANE_HELPER_NO_SCALING;
@@ -12862,8 +12869,10 @@ static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
 	 *            or
 	 *    cdclk/crtc_clock
 	 */
-	max_scale = min((1 << 16) * 3 - 1,
-			(1 << 8) * ((max_dotclk << 8) / crtc_clock));
+	mult = pixel_format == DRM_FORMAT_NV12 ? 2 : 3;
+	tmpclk1 = (1 << 16) * mult - 1;
+	tmpclk2 = (1 << 8) * ((max_dotclk << 8) / crtc_clock);
+	max_scale = min(tmpclk1, tmpclk2);
 
 	return max_scale;
 }
@@ -12884,7 +12893,11 @@ static void add_rps_boost_after_vblank(struct drm_crtc *crtc,
 		/* use scaler when colorkey is not required */
 		if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
 			min_scale = 1;
-			max_scale = skl_max_scale(to_intel_crtc(crtc), crtc_state);
+			max_scale = skl_max_scale(to_intel_crtc(crtc),
+						crtc_state,
+						state->base.fb ?
+						state->base.fb->format->format :
+						0);
 		}
 		can_position = true;
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 17649f1..aeafdea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1457,7 +1457,8 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
-int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
+int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
+	uint32_t pixel_format);
 
 static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
 {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 54f876e..47e5ba9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -818,7 +818,8 @@ static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
 		if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
 			can_scale = 1;
 			min_scale = 1;
-			max_scale = skl_max_scale(crtc, crtc_state);
+			max_scale = skl_max_scale(crtc, crtc_state,
+						fb->format->format);
 		} else {
 			can_scale = 0;
 			min_scale = DRM_PLANE_HELPER_NO_SCALING;
-- 
1.9.1

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

[PATCH 4/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)

Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4e73d88..6cf8806 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -106,6 +106,22 @@
 	DRM_FORMAT_MOD_INVALID
 };
 
+static const uint32_t nv12_primary_formats[] = {
+	DRM_FORMAT_C8,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
+};
+
 /* Cursor formats */
 static const uint32_t intel_cursor_formats[] = {
 	DRM_FORMAT_ARGB8888,
@@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
 		primary->update_plane = skylake_update_primary_plane;
 		primary->disable_plane = skylake_disable_primary_plane;
 	} else if (INTEL_GEN(dev_priv) >= 9) {
-		intel_primary_formats = skl_primary_formats;
-		num_formats = ARRAY_SIZE(skl_primary_formats);
+		if (IS_BROXTON(dev_priv) &&
+			((pipe == PIPE_A || pipe == PIPE_B))) {
+			intel_primary_formats = nv12_primary_formats;
+			num_formats = ARRAY_SIZE(nv12_primary_formats);
+		} else {
+			intel_primary_formats = skl_primary_formats;
+			num_formats = ARRAY_SIZE(skl_primary_formats);
+		}
 		if (pipe < PIPE_C)
 			modifiers = skl_format_modifiers_ccs;
 		else
-- 
1.9.1

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

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

[Matt Roper]

On Mon, Aug 28, 2017 at 04:22:20PM +0530, Vidya Srinivas wrote:
> 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)
> 
> Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4e73d88..6cf8806 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -106,6 +106,22 @@
>  	DRM_FORMAT_MOD_INVALID
>  };
>  
> +static const uint32_t nv12_primary_formats[] = {
> +	DRM_FORMAT_C8,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
> +	DRM_FORMAT_YUYV,
> +	DRM_FORMAT_YVYU,
> +	DRM_FORMAT_UYVY,
> +	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
> +};
> +
>  /* Cursor formats */
>  static const uint32_t intel_cursor_formats[] = {
>  	DRM_FORMAT_ARGB8888,
> @@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
>  		primary->update_plane = skylake_update_primary_plane;
>  		primary->disable_plane = skylake_disable_primary_plane;
>  	} else if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_primary_formats = skl_primary_formats;
> -		num_formats = ARRAY_SIZE(skl_primary_formats);
> +		if (IS_BROXTON(dev_priv) &&

I believe this needs to be

   if (IS_BXT_REVID(dev_priv, BXT_REVID_D0, BXT_REVID_FOREVER) ...

There were unavoidable flickering/underrun issues on the earlier
steppings due to memory fetch issues for the second color plane.  Those
issues were only fixed on the E0 SoC stepping (which incorporates the D0
Display/GT).

Same change for your sprite plane changes in the next patch.


Matt

> +			((pipe == PIPE_A || pipe == PIPE_B))) {
> +			intel_primary_formats = nv12_primary_formats;
> +			num_formats = ARRAY_SIZE(nv12_primary_formats);
> +		} else {
> +			intel_primary_formats = skl_primary_formats;
> +			num_formats = ARRAY_SIZE(skl_primary_formats);
> +		}
>  		if (pipe < PIPE_C)
>  			modifiers = skl_format_modifiers_ccs;
>  		else
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rodrigo Vivi]

On Wed, Sep 06, 2017 at 09:36:27AM -0700, Matt Roper wrote:
> On Mon, Aug 28, 2017 at 04:22:20PM +0530, Vidya Srinivas wrote:
> > 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)
> > 
> > Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 26 ++++++++++++++++++++++++--
> >  1 file changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 4e73d88..6cf8806 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -106,6 +106,22 @@
> >  	DRM_FORMAT_MOD_INVALID
> >  };
> >  
> > +static const uint32_t nv12_primary_formats[] = {
> > +	DRM_FORMAT_C8,
> > +	DRM_FORMAT_RGB565,
> > +	DRM_FORMAT_XRGB8888,
> > +	DRM_FORMAT_XBGR8888,
> > +	DRM_FORMAT_ARGB8888,
> > +	DRM_FORMAT_ABGR8888,
> > +	DRM_FORMAT_XRGB2101010,
> > +	DRM_FORMAT_XBGR2101010,
> > +	DRM_FORMAT_YUYV,
> > +	DRM_FORMAT_YVYU,
> > +	DRM_FORMAT_UYVY,
> > +	DRM_FORMAT_VYUY,
> > +	DRM_FORMAT_NV12,
> > +};
> > +
> >  /* Cursor formats */
> >  static const uint32_t intel_cursor_formats[] = {
> >  	DRM_FORMAT_ARGB8888,
> > @@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
> >  		primary->update_plane = skylake_update_primary_plane;
> >  		primary->disable_plane = skylake_disable_primary_plane;
> >  	} else if (INTEL_GEN(dev_priv) >= 9) {
> > -		intel_primary_formats = skl_primary_formats;
> > -		num_formats = ARRAY_SIZE(skl_primary_formats);
> > +		if (IS_BROXTON(dev_priv) &&
> 
> I believe this needs to be
> 
>    if (IS_BXT_REVID(dev_priv, BXT_REVID_D0, BXT_REVID_FOREVER) ...

We usually use this stepping information for Workarounds. So usually
blocks around this are the non-expected default behaviour.
So I'd handle that from A0 to C0 and else the default behaviour or at least
![A0,C0]...

> 
> There were unavoidable flickering/underrun issues on the earlier
> steppings due to memory fetch issues for the second color plane.  Those
> issues were only fixed on the E0 SoC stepping (which incorporates the D0
> Display/GT).

Also we usuallly use this steppings checking with a documented W/a.
Is there one? Anything that would justify this?

But also, is there any team there still using anything older than D0? yet?

If we don't know anyone probably pure IS_BROXTON is the best option.

> 
> Same change for your sprite plane changes in the next patch.
> 
> 
> Matt
> 
> > +			((pipe == PIPE_A || pipe == PIPE_B))) {
> > +			intel_primary_formats = nv12_primary_formats;
> > +			num_formats = ARRAY_SIZE(nv12_primary_formats);
> > +		} else {
> > +			intel_primary_formats = skl_primary_formats;
> > +			num_formats = ARRAY_SIZE(skl_primary_formats);
> > +		}
> >  		if (pipe < PIPE_C)
> >  			modifiers = skl_format_modifiers_ccs;
> >  		else
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Matt Roper]

On Wed, Sep 06, 2017 at 12:12:10PM -0700, Rodrigo Vivi wrote:
> On Wed, Sep 06, 2017 at 09:36:27AM -0700, Matt Roper wrote:
> > On Mon, Aug 28, 2017 at 04:22:20PM +0530, Vidya Srinivas wrote:
> > > 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)
> > > 
> > > Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 26 ++++++++++++++++++++++++--
> > >  1 file changed, 24 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 4e73d88..6cf8806 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -106,6 +106,22 @@
> > >  	DRM_FORMAT_MOD_INVALID
> > >  };
> > >  
> > > +static const uint32_t nv12_primary_formats[] = {
> > > +	DRM_FORMAT_C8,
> > > +	DRM_FORMAT_RGB565,
> > > +	DRM_FORMAT_XRGB8888,
> > > +	DRM_FORMAT_XBGR8888,
> > > +	DRM_FORMAT_ARGB8888,
> > > +	DRM_FORMAT_ABGR8888,
> > > +	DRM_FORMAT_XRGB2101010,
> > > +	DRM_FORMAT_XBGR2101010,
> > > +	DRM_FORMAT_YUYV,
> > > +	DRM_FORMAT_YVYU,
> > > +	DRM_FORMAT_UYVY,
> > > +	DRM_FORMAT_VYUY,
> > > +	DRM_FORMAT_NV12,
> > > +};
> > > +
> > >  /* Cursor formats */
> > >  static const uint32_t intel_cursor_formats[] = {
> > >  	DRM_FORMAT_ARGB8888,
> > > @@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
> > >  		primary->update_plane = skylake_update_primary_plane;
> > >  		primary->disable_plane = skylake_disable_primary_plane;
> > >  	} else if (INTEL_GEN(dev_priv) >= 9) {
> > > -		intel_primary_formats = skl_primary_formats;
> > > -		num_formats = ARRAY_SIZE(skl_primary_formats);
> > > +		if (IS_BROXTON(dev_priv) &&
> > 
> > I believe this needs to be
> > 
> >    if (IS_BXT_REVID(dev_priv, BXT_REVID_D0, BXT_REVID_FOREVER) ...
> 
> We usually use this stepping information for Workarounds. So usually
> blocks around this are the non-expected default behaviour.
> So I'd handle that from A0 to C0 and else the default behaviour or at least
> ![A0,C0]...
> 
> > 
> > There were unavoidable flickering/underrun issues on the earlier
> > steppings due to memory fetch issues for the second color plane.  Those
> > issues were only fixed on the E0 SoC stepping (which incorporates the D0
> > Display/GT).
> 
> Also we usuallly use this steppings checking with a documented W/a.
> Is there one? Anything that would justify this?

Unfortunately the bspec WA database still hasn't been updated to
indicate that *any* SKU of can properly support NV12.  So the workaround
database still just gives a general "don't use NV12 at all" statement
(entry 0870 in the display WA database which is listed for "BXT:ALL").
I tried unravel what the internal communication channels are for this
kind of update a few months ago, but didn't make much headway.

> 
> But also, is there any team there still using anything older than D0? yet?

Yes, definitely.  Pre-E0 SoC's (and thus pre-D0 graphics) is what a lot
of our embedded customers have already gone to production with.


Matt


> 
> If we don't know anyone probably pure IS_BROXTON is the best option.
> 
> > 
> > Same change for your sprite plane changes in the next patch.
> > 
> > 
> > Matt
> > 
> > > +			((pipe == PIPE_A || pipe == PIPE_B))) {
> > > +			intel_primary_formats = nv12_primary_formats;
> > > +			num_formats = ARRAY_SIZE(nv12_primary_formats);
> > > +		} else {
> > > +			intel_primary_formats = skl_primary_formats;
> > > +			num_formats = ARRAY_SIZE(skl_primary_formats);
> > > +		}
> > >  		if (pipe < PIPE_C)
> > >  			modifiers = skl_format_modifiers_ccs;
> > >  		else
> > > -- 
> > > 1.9.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > IoTG Platform Enabling & Development
> > Intel Corporation
> > (916) 356-2795
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Kristian Høgsberg]

On Wed, Sep 6, 2017 at 1:17 PM, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Wed, Sep 06, 2017 at 12:12:10PM -0700, Rodrigo Vivi wrote:
>> On Wed, Sep 06, 2017 at 09:36:27AM -0700, Matt Roper wrote:
>> > On Mon, Aug 28, 2017 at 04:22:20PM +0530, Vidya Srinivas wrote:
>> > > 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)
>> > >
>> > > Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 26 ++++++++++++++++++++++++--
>> > >  1 file changed, 24 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> > > index 4e73d88..6cf8806 100644
>> > > --- a/drivers/gpu/drm/i915/intel_display.c
>> > > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > > @@ -106,6 +106,22 @@
>> > >   DRM_FORMAT_MOD_INVALID
>> > >  };
>> > >
>> > > +static const uint32_t nv12_primary_formats[] = {
>> > > + DRM_FORMAT_C8,
>> > > + DRM_FORMAT_RGB565,
>> > > + DRM_FORMAT_XRGB8888,
>> > > + DRM_FORMAT_XBGR8888,
>> > > + DRM_FORMAT_ARGB8888,
>> > > + DRM_FORMAT_ABGR8888,
>> > > + DRM_FORMAT_XRGB2101010,
>> > > + DRM_FORMAT_XBGR2101010,
>> > > + DRM_FORMAT_YUYV,
>> > > + DRM_FORMAT_YVYU,
>> > > + DRM_FORMAT_UYVY,
>> > > + DRM_FORMAT_VYUY,
>> > > + DRM_FORMAT_NV12,
>> > > +};
>> > > +
>> > >  /* Cursor formats */
>> > >  static const uint32_t intel_cursor_formats[] = {
>> > >   DRM_FORMAT_ARGB8888,
>> > > @@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
>> > >           primary->update_plane = skylake_update_primary_plane;
>> > >           primary->disable_plane = skylake_disable_primary_plane;
>> > >   } else if (INTEL_GEN(dev_priv) >= 9) {
>> > > -         intel_primary_formats = skl_primary_formats;
>> > > -         num_formats = ARRAY_SIZE(skl_primary_formats);
>> > > +         if (IS_BROXTON(dev_priv) &&
>> >
>> > I believe this needs to be
>> >
>> >    if (IS_BXT_REVID(dev_priv, BXT_REVID_D0, BXT_REVID_FOREVER) ...
>>
>> We usually use this stepping information for Workarounds. So usually
>> blocks around this are the non-expected default behaviour.
>> So I'd handle that from A0 to C0 and else the default behaviour or at least
>> ![A0,C0]...
>>
>> >
>> > There were unavoidable flickering/underrun issues on the earlier
>> > steppings due to memory fetch issues for the second color plane.  Those
>> > issues were only fixed on the E0 SoC stepping (which incorporates the D0
>> > Display/GT).
>>
>> Also we usuallly use this steppings checking with a documented W/a.
>> Is there one? Anything that would justify this?
>
> Unfortunately the bspec WA database still hasn't been updated to
> indicate that *any* SKU of can properly support NV12.  So the workaround
> database still just gives a general "don't use NV12 at all" statement
> (entry 0870 in the display WA database which is listed for "BXT:ALL").
> I tried unravel what the internal communication channels are for this
> kind of update a few months ago, but didn't make much headway.

What about KBL support?

>>
>> But also, is there any team there still using anything older than D0? yet?
>
> Yes, definitely.  Pre-E0 SoC's (and thus pre-D0 graphics) is what a lot
> of our embedded customers have already gone to production with.
>
>
> Matt
>
>
>>
>> If we don't know anyone probably pure IS_BROXTON is the best option.
>>
>> >
>> > Same change for your sprite plane changes in the next patch.
>> >
>> >
>> > Matt
>> >
>> > > +                 ((pipe == PIPE_A || pipe == PIPE_B))) {
>> > > +                 intel_primary_formats = nv12_primary_formats;
>> > > +                 num_formats = ARRAY_SIZE(nv12_primary_formats);
>> > > +         } else {
>> > > +                 intel_primary_formats = skl_primary_formats;
>> > > +                 num_formats = ARRAY_SIZE(skl_primary_formats);
>> > > +         }
>> > >           if (pipe < PIPE_C)
>> > >                   modifiers = skl_format_modifiers_ccs;
>> > >           else
>> > > --
>> > > 1.9.1
>> > >
>> > > _______________________________________________
>> > > Intel-gfx mailing list
>> > > Intel-gfx@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > --
>> > Matt Roper
>> > Graphics Software Engineer
>> > IoTG Platform Enabling & Development
>> > Intel Corporation
>> > (916) 356-2795
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Matt Roper]

On Wed, Sep 06, 2017 at 02:49:07PM -0700, Kristian Høgsberg wrote:
> On Wed, Sep 6, 2017 at 1:17 PM, Matt Roper <matthew.d.roper@intel.com> wrote:
> > On Wed, Sep 06, 2017 at 12:12:10PM -0700, Rodrigo Vivi wrote:
> >> On Wed, Sep 06, 2017 at 09:36:27AM -0700, Matt Roper wrote:
> >> > On Mon, Aug 28, 2017 at 04:22:20PM +0530, Vidya Srinivas wrote:
> >> > > 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)
> >> > >
> >> > > Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 26 ++++++++++++++++++++++++--
> >> > >  1 file changed, 24 insertions(+), 2 deletions(-)
> >> > >
> >> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> > > index 4e73d88..6cf8806 100644
> >> > > --- a/drivers/gpu/drm/i915/intel_display.c
> >> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> >> > > @@ -106,6 +106,22 @@
> >> > >   DRM_FORMAT_MOD_INVALID
> >> > >  };
> >> > >
> >> > > +static const uint32_t nv12_primary_formats[] = {
> >> > > + DRM_FORMAT_C8,
> >> > > + DRM_FORMAT_RGB565,
> >> > > + DRM_FORMAT_XRGB8888,
> >> > > + DRM_FORMAT_XBGR8888,
> >> > > + DRM_FORMAT_ARGB8888,
> >> > > + DRM_FORMAT_ABGR8888,
> >> > > + DRM_FORMAT_XRGB2101010,
> >> > > + DRM_FORMAT_XBGR2101010,
> >> > > + DRM_FORMAT_YUYV,
> >> > > + DRM_FORMAT_YVYU,
> >> > > + DRM_FORMAT_UYVY,
> >> > > + DRM_FORMAT_VYUY,
> >> > > + DRM_FORMAT_NV12,
> >> > > +};
> >> > > +
> >> > >  /* Cursor formats */
> >> > >  static const uint32_t intel_cursor_formats[] = {
> >> > >   DRM_FORMAT_ARGB8888,
> >> > > @@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
> >> > >           primary->update_plane = skylake_update_primary_plane;
> >> > >           primary->disable_plane = skylake_disable_primary_plane;
> >> > >   } else if (INTEL_GEN(dev_priv) >= 9) {
> >> > > -         intel_primary_formats = skl_primary_formats;
> >> > > -         num_formats = ARRAY_SIZE(skl_primary_formats);
> >> > > +         if (IS_BROXTON(dev_priv) &&
> >> >
> >> > I believe this needs to be
> >> >
> >> >    if (IS_BXT_REVID(dev_priv, BXT_REVID_D0, BXT_REVID_FOREVER) ...
> >>
> >> We usually use this stepping information for Workarounds. So usually
> >> blocks around this are the non-expected default behaviour.
> >> So I'd handle that from A0 to C0 and else the default behaviour or at least
> >> ![A0,C0]...
> >>
> >> >
> >> > There were unavoidable flickering/underrun issues on the earlier
> >> > steppings due to memory fetch issues for the second color plane.  Those
> >> > issues were only fixed on the E0 SoC stepping (which incorporates the D0
> >> > Display/GT).
> >>
> >> Also we usuallly use this steppings checking with a documented W/a.
> >> Is there one? Anything that would justify this?
> >
> > Unfortunately the bspec WA database still hasn't been updated to
> > indicate that *any* SKU of can properly support NV12.  So the workaround
> > database still just gives a general "don't use NV12 at all" statement
> > (entry 0870 in the display WA database which is listed for "BXT:ALL").

Woops, this is actually 0826, not 0870 (0870 is a different NV12 entry
specifically for y-tile).


> > I tried unravel what the internal communication channels are for this
> > kind of update a few months ago, but didn't make much headway.
> 
> What about KBL support?
> 

The only NV12 workaround I see for KBL is that KBL A-step and B-step
shouldn't do NV12 + ytile.


Matt


> >>
> >> But also, is there any team there still using anything older than D0? yet?
> >
> > Yes, definitely.  Pre-E0 SoC's (and thus pre-D0 graphics) is what a lot
> > of our embedded customers have already gone to production with.
> >
> >
> > Matt
> >
> >
> >>
> >> If we don't know anyone probably pure IS_BROXTON is the best option.
> >>
> >> >
> >> > Same change for your sprite plane changes in the next patch.
> >> >
> >> >
> >> > Matt
> >> >
> >> > > +                 ((pipe == PIPE_A || pipe == PIPE_B))) {
> >> > > +                 intel_primary_formats = nv12_primary_formats;
> >> > > +                 num_formats = ARRAY_SIZE(nv12_primary_formats);
> >> > > +         } else {
> >> > > +                 intel_primary_formats = skl_primary_formats;
> >> > > +                 num_formats = ARRAY_SIZE(skl_primary_formats);
> >> > > +         }
> >> > >           if (pipe < PIPE_C)
> >> > >                   modifiers = skl_format_modifiers_ccs;
> >> > >           else
> >> > > --
> >> > > 1.9.1
> >> > >
> >> > > _______________________________________________
> >> > > Intel-gfx mailing list
> >> > > Intel-gfx@lists.freedesktop.org
> >> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> > --
> >> > Matt Roper
> >> > Graphics Software Engineer
> >> > IoTG Platform Enabling & Development
> >> > Intel Corporation
> >> > (916) 356-2795
> >> > _______________________________________________
> >> > Intel-gfx mailing list
> >> > Intel-gfx@lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Matt Roper
> > Graphics Software Engineer
> > IoTG Platform Enabling & Development
> > Intel Corporation
> > (916) 356-2795
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Kristian Høgsberg]

On Wed, Sep 6, 2017 at 3:02 PM, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Wed, Sep 06, 2017 at 02:49:07PM -0700, Kristian Høgsberg wrote:
>> On Wed, Sep 6, 2017 at 1:17 PM, Matt Roper <matthew.d.roper@intel.com> wrote:
>> > On Wed, Sep 06, 2017 at 12:12:10PM -0700, Rodrigo Vivi wrote:
>> >> On Wed, Sep 06, 2017 at 09:36:27AM -0700, Matt Roper wrote:
>> >> > On Mon, Aug 28, 2017 at 04:22:20PM +0530, Vidya Srinivas wrote:
>> >> > > 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)
>> >> > >
>> >> > > Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 26 ++++++++++++++++++++++++--
>> >> > >  1 file changed, 24 insertions(+), 2 deletions(-)
>> >> > >
>> >> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> >> > > index 4e73d88..6cf8806 100644
>> >> > > --- a/drivers/gpu/drm/i915/intel_display.c
>> >> > > +++ b/drivers/gpu/drm/i915/intel_display.c
>> >> > > @@ -106,6 +106,22 @@
>> >> > >   DRM_FORMAT_MOD_INVALID
>> >> > >  };
>> >> > >
>> >> > > +static const uint32_t nv12_primary_formats[] = {
>> >> > > + DRM_FORMAT_C8,
>> >> > > + DRM_FORMAT_RGB565,
>> >> > > + DRM_FORMAT_XRGB8888,
>> >> > > + DRM_FORMAT_XBGR8888,
>> >> > > + DRM_FORMAT_ARGB8888,
>> >> > > + DRM_FORMAT_ABGR8888,
>> >> > > + DRM_FORMAT_XRGB2101010,
>> >> > > + DRM_FORMAT_XBGR2101010,
>> >> > > + DRM_FORMAT_YUYV,
>> >> > > + DRM_FORMAT_YVYU,
>> >> > > + DRM_FORMAT_UYVY,
>> >> > > + DRM_FORMAT_VYUY,
>> >> > > + DRM_FORMAT_NV12,
>> >> > > +};
>> >> > > +
>> >> > >  /* Cursor formats */
>> >> > >  static const uint32_t intel_cursor_formats[] = {
>> >> > >   DRM_FORMAT_ARGB8888,
>> >> > > @@ -13280,8 +13296,14 @@ static bool intel_cursor_plane_format_mod_supported(struct drm_plane *plane,
>> >> > >           primary->update_plane = skylake_update_primary_plane;
>> >> > >           primary->disable_plane = skylake_disable_primary_plane;
>> >> > >   } else if (INTEL_GEN(dev_priv) >= 9) {
>> >> > > -         intel_primary_formats = skl_primary_formats;
>> >> > > -         num_formats = ARRAY_SIZE(skl_primary_formats);
>> >> > > +         if (IS_BROXTON(dev_priv) &&
>> >> >
>> >> > I believe this needs to be
>> >> >
>> >> >    if (IS_BXT_REVID(dev_priv, BXT_REVID_D0, BXT_REVID_FOREVER) ...
>> >>
>> >> We usually use this stepping information for Workarounds. So usually
>> >> blocks around this are the non-expected default behaviour.
>> >> So I'd handle that from A0 to C0 and else the default behaviour or at least
>> >> ![A0,C0]...
>> >>
>> >> >
>> >> > There were unavoidable flickering/underrun issues on the earlier
>> >> > steppings due to memory fetch issues for the second color plane.  Those
>> >> > issues were only fixed on the E0 SoC stepping (which incorporates the D0
>> >> > Display/GT).
>> >>
>> >> Also we usuallly use this steppings checking with a documented W/a.
>> >> Is there one? Anything that would justify this?
>> >
>> > Unfortunately the bspec WA database still hasn't been updated to
>> > indicate that *any* SKU of can properly support NV12.  So the workaround
>> > database still just gives a general "don't use NV12 at all" statement
>> > (entry 0870 in the display WA database which is listed for "BXT:ALL").
>
> Woops, this is actually 0826, not 0870 (0870 is a different NV12 entry
> specifically for y-tile).
>
>
>> > I tried unravel what the internal communication channels are for this
>> > kind of update a few months ago, but didn't make much headway.
>>
>> What about KBL support?
>>
>
> The only NV12 workaround I see for KBL is that KBL A-step and B-step
> shouldn't do NV12 + ytile.

But does this series actually enable KBL NV12 overlays? I only see
IS_BROXTON() in the conditional - that doesn't include KBL, does it?

> Matt
>
>
>> >>
>> >> But also, is there any team there still using anything older than D0? yet?
>> >
>> > Yes, definitely.  Pre-E0 SoC's (and thus pre-D0 graphics) is what a lot
>> > of our embedded customers have already gone to production with.
>> >
>> >
>> > Matt
>> >
>> >
>> >>
>> >> If we don't know anyone probably pure IS_BROXTON is the best option.
>> >>
>> >> >
>> >> > Same change for your sprite plane changes in the next patch.
>> >> >
>> >> >
>> >> > Matt
>> >> >
>> >> > > +                 ((pipe == PIPE_A || pipe == PIPE_B))) {
>> >> > > +                 intel_primary_formats = nv12_primary_formats;
>> >> > > +                 num_formats = ARRAY_SIZE(nv12_primary_formats);
>> >> > > +         } else {
>> >> > > +                 intel_primary_formats = skl_primary_formats;
>> >> > > +                 num_formats = ARRAY_SIZE(skl_primary_formats);
>> >> > > +         }
>> >> > >           if (pipe < PIPE_C)
>> >> > >                   modifiers = skl_format_modifiers_ccs;
>> >> > >           else
>> >> > > --
>> >> > > 1.9.1
>> >> > >
>> >> > > _______________________________________________
>> >> > > Intel-gfx mailing list
>> >> > > Intel-gfx@lists.freedesktop.org
>> >> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> >
>> >> > --
>> >> > Matt Roper
>> >> > Graphics Software Engineer
>> >> > IoTG Platform Enabling & Development
>> >> > Intel Corporation
>> >> > (916) 356-2795
>> >> > _______________________________________________
>> >> > Intel-gfx mailing list
>> >> > Intel-gfx@lists.freedesktop.org
>> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > --
>> > Matt Roper
>> > Graphics Software Engineer
>> > IoTG Platform Enabling & Development
>> > Intel Corporation
>> > (916) 356-2795
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/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)

Tested-by: Clinton Taylor <clinton.a.taylor@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 | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 47e5ba9..5b49e4e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1197,6 +1197,19 @@ static bool intel_sprite_plane_format_mod_supported(struct drm_plane *plane,
         .format_mod_supported = intel_sprite_plane_format_mod_supported,
 };
 
+static uint32_t nv12_plane_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,
+};
+
 struct intel_plane *
 intel_sprite_plane_create(struct drm_i915_private *dev_priv,
 			  enum pipe pipe, int plane)
@@ -1239,9 +1252,14 @@ struct intel_plane *
 
 		intel_plane->update_plane = skl_update_plane;
 		intel_plane->disable_plane = skl_disable_plane;
-
-		plane_formats = skl_plane_formats;
-		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		if (IS_BROXTON(dev_priv) &&
+			(pipe == PIPE_A || pipe == PIPE_B)) {
+			plane_formats = nv12_plane_formats;
+			num_plane_formats = ARRAY_SIZE(nv12_plane_formats);
+		} else {
+			plane_formats = skl_plane_formats;
+			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		}
 		modifiers = skl_plane_format_modifiers;
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		intel_plane->can_scale = false;
-- 
1.9.1

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

[PATCH 6/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)

Tested-by: Clinton Taylor <clinton.a.taylor@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_display.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6cf8806..615c7cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14095,6 +14095,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 			goto err;
 		}
 		break;
+	case DRM_FORMAT_NV12:
+		if (!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));
-- 
1.9.1

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

✗ Fi.CI.BAT: failure for Adding NV12 support (rev2)

[Patchwork]

== Series Details ==

Series: Adding NV12 support (rev2)
URL   : https://patchwork.freedesktop.org/series/28103/
State : failure

== Summary ==

Series 28103v2 Adding NV12 support
https://patchwork.freedesktop.org/api/1.0/series/28103/revisions/2/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-kbl-7500u)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:452s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:435s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:368s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:556s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:234s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:524s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:520s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:516s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:440s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:610s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:453s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:423s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:502s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:477s
fi-kbl-7500u     total:238  pass:222  dwarn:0   dfail:0   fail:0   skip:15 
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:595s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:595s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:525s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:462s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:487s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:443s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:500s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:544s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:405s

00b77f621f835bc114b79ec897b9aa277ea5726b drm-tip: 2017y-08m-28d-10h-25m-08s UTC integration manifest
149fc1e88a51 drm/i915: Add NV12 support to intel_framebuffer_init
26135eea1a26 drm/i915: Add NV12 as supported format for sprite plane
2083ffe043b7 drm/i915: Add NV12 as supported format for primary plane
93ab85476653 drm/i915: Upscale scaler max scale for NV12
d291f3965561 drm/i915: Update format_is_yuv() to include NV12
b6c629b41415 drm/i915: Set scaler mode for NV12

== Logs ==

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

Re: [PATCH 0/6] Adding NV12 support

[Daniel Vetter]

On Mon, Aug 28, 2017 at 04:22:16PM +0530, Vidya Srinivas wrote:
> This patch series is adding NV12 support for Broxton display after
> rebasing on latest drm-intel-nightly. Initial series of the patches
> can be found here:
> https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.html
> 
> Previous revision history:
> Patches were initial reviewed last when floated but
> currently there was a design change with respect to
> - the way fb offset is handled
> - the way rotation is handled
> Rebase of the current NV12 patch series has been done as per the
> current changes on drm-intel-nightly.
> Review comments from Ville (12th June 2017) have been addressed
> Review comments from Clinton A Taylor (7th July 2017) have been
> addressed
> Review comments from Clinton A Taylor (10th July 2017) have been
> addressed. Had missed out tested-by/reviewed-by in the patches.
> Fixed that error in this series.
> Review comments from Ville (11th July 2017) addressed.
> Review comments from Paauwe, Bob (29th July 2017) addressed.
> 
> Update from last rev:
> Rebased the series as Ville's patches are merged. Previously,
> this series included those floating patches.
> 
> Chandra Konduru (6):
>   drm/i915: Set scaler mode for NV12
>   drm/i915: Update format_is_yuv() to include NV12
>   drm/i915: Upscale scaler max scale for NV12
>   drm/i915: Add NV12 as supported format for primary plane
>   drm/i915: Add NV12 as supported format for sprite plane
>   drm/i915: Add NV12 support to intel_framebuffer_init

Needs serious work on the plane scaling igt (it's atm all broken, and
doesn't test any atomic interactions).

Then this needs serious work on the nv12 plane igts (which don't yet
exist).

Then this probably needs pile more igts to test interactions between
everything (e.g. rotation, ...).

In short: This needs itgs. Lots of them :-)

Before those exist, and before we've tracked down the bug in the existing
code you're building on it imo makes no sense to start reviewing these
here.

Thanks, Daniel

> 
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_atomic.c  |  8 ++++-
>  drivers/gpu/drm/i915/intel_display.c | 67 +++++++++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
>  drivers/gpu/drm/i915/intel_sprite.c  | 34 ++++++++++++++----
>  5 files changed, 92 insertions(+), 21 deletions(-)
> 
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Adding NV12 support

[Kristian Høgsberg]

On Mon, Aug 28, 2017 at 3:17 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Aug 28, 2017 at 04:22:16PM +0530, Vidya Srinivas wrote:
>> This patch series is adding NV12 support for Broxton display after
>> rebasing on latest drm-intel-nightly. Initial series of the patches
>> can be found here:
>> https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.html
>>
>> Previous revision history:
>> Patches were initial reviewed last when floated but
>> currently there was a design change with respect to
>> - the way fb offset is handled
>> - the way rotation is handled
>> Rebase of the current NV12 patch series has been done as per the
>> current changes on drm-intel-nightly.
>> Review comments from Ville (12th June 2017) have been addressed
>> Review comments from Clinton A Taylor (7th July 2017) have been
>> addressed
>> Review comments from Clinton A Taylor (10th July 2017) have been
>> addressed. Had missed out tested-by/reviewed-by in the patches.
>> Fixed that error in this series.
>> Review comments from Ville (11th July 2017) addressed.
>> Review comments from Paauwe, Bob (29th July 2017) addressed.
>>
>> Update from last rev:
>> Rebased the series as Ville's patches are merged. Previously,
>> this series included those floating patches.
>>
>> Chandra Konduru (6):
>>   drm/i915: Set scaler mode for NV12
>>   drm/i915: Update format_is_yuv() to include NV12
>>   drm/i915: Upscale scaler max scale for NV12
>>   drm/i915: Add NV12 as supported format for primary plane
>>   drm/i915: Add NV12 as supported format for sprite plane
>>   drm/i915: Add NV12 support to intel_framebuffer_init
>
> Needs serious work on the plane scaling igt (it's atm all broken, and
> doesn't test any atomic interactions).
>
> Then this needs serious work on the nv12 plane igts (which don't yet
> exist).
>
> Then this probably needs pile more igts to test interactions between
> everything (e.g. rotation, ...).
>
> In short: This needs itgs. Lots of them :-)
>
> Before those exist, and before we've tracked down the bug in the existing
> code you're building on it imo makes no sense to start reviewing these
> here.

Also, this series need to advertise which modifiers work with the new
NV12 format by adding a case to
skl_sprite_plane_format_mod_supported() and skl_mod_supported().

Kristian

> Thanks, Daniel
>
>>
>>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>>  drivers/gpu/drm/i915/intel_atomic.c  |  8 ++++-
>>  drivers/gpu/drm/i915/intel_display.c | 67 +++++++++++++++++++++++++++++-------
>>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
>>  drivers/gpu/drm/i915/intel_sprite.c  | 34 ++++++++++++++----
>>  5 files changed, 92 insertions(+), 21 deletions(-)
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Adding NV12 support

[Daniel Vetter]

On Tue, Sep 19, 2017 at 02:56:17PM -0700, Kristian Høgsberg wrote:
> On Mon, Aug 28, 2017 at 3:17 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Aug 28, 2017 at 04:22:16PM +0530, Vidya Srinivas wrote:
> >> This patch series is adding NV12 support for Broxton display after
> >> rebasing on latest drm-intel-nightly. Initial series of the patches
> >> can be found here:
> >> https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.html
> >>
> >> Previous revision history:
> >> Patches were initial reviewed last when floated but
> >> currently there was a design change with respect to
> >> - the way fb offset is handled
> >> - the way rotation is handled
> >> Rebase of the current NV12 patch series has been done as per the
> >> current changes on drm-intel-nightly.
> >> Review comments from Ville (12th June 2017) have been addressed
> >> Review comments from Clinton A Taylor (7th July 2017) have been
> >> addressed
> >> Review comments from Clinton A Taylor (10th July 2017) have been
> >> addressed. Had missed out tested-by/reviewed-by in the patches.
> >> Fixed that error in this series.
> >> Review comments from Ville (11th July 2017) addressed.
> >> Review comments from Paauwe, Bob (29th July 2017) addressed.
> >>
> >> Update from last rev:
> >> Rebased the series as Ville's patches are merged. Previously,
> >> this series included those floating patches.
> >>
> >> Chandra Konduru (6):
> >>   drm/i915: Set scaler mode for NV12
> >>   drm/i915: Update format_is_yuv() to include NV12
> >>   drm/i915: Upscale scaler max scale for NV12
> >>   drm/i915: Add NV12 as supported format for primary plane
> >>   drm/i915: Add NV12 as supported format for sprite plane
> >>   drm/i915: Add NV12 support to intel_framebuffer_init
> >
> > Needs serious work on the plane scaling igt (it's atm all broken, and
> > doesn't test any atomic interactions).
> >
> > Then this needs serious work on the nv12 plane igts (which don't yet
> > exist).
> >
> > Then this probably needs pile more igts to test interactions between
> > everything (e.g. rotation, ...).
> >
> > In short: This needs itgs. Lots of them :-)
> >
> > Before those exist, and before we've tracked down the bug in the existing
> > code you're building on it imo makes no sense to start reviewing these
> > here.
> 
> Also, this series need to advertise which modifiers work with the new
> NV12 format by adding a case to
> skl_sprite_plane_format_mod_supported() and skl_mod_supported().

Oh right, which also means we need igts to test the plane formats against
more modifiers! Well that's kinda part of the tiling tests I guess, at
least for nv12.
-Daniel
> 
> Kristian
> 
> > Thanks, Daniel
> >
> >>
> >>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
> >>  drivers/gpu/drm/i915/intel_atomic.c  |  8 ++++-
> >>  drivers/gpu/drm/i915/intel_display.c | 67 +++++++++++++++++++++++++++++-------
> >>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
> >>  drivers/gpu/drm/i915/intel_sprite.c  | 34 ++++++++++++++----
> >>  5 files changed, 92 insertions(+), 21 deletions(-)
> >>
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/6] Adding NV12 support

[Srinivas, Vidya]

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Tuesday, September 26, 2017 5:17 PM
> To: Kristian Høgsberg <hoegsberg@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>; Srinivas, Vidya
> <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 0/6] Adding NV12 support
> 
> On Tue, Sep 19, 2017 at 02:56:17PM -0700, Kristian Høgsberg wrote:
> > On Mon, Aug 28, 2017 at 3:17 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > > On Mon, Aug 28, 2017 at 04:22:16PM +0530, Vidya Srinivas wrote:
> > >> This patch series is adding NV12 support for Broxton display after
> > >> rebasing on latest drm-intel-nightly. Initial series of the patches
> > >> can be found here:
> > >> https://lists.freedesktop.org/archives/intel-gfx/2015-May/066786.ht
> > >> ml
> > >>
> > >> Previous revision history:
> > >> Patches were initial reviewed last when floated but currently there
> > >> was a design change with respect to
> > >> - the way fb offset is handled
> > >> - the way rotation is handled
> > >> Rebase of the current NV12 patch series has been done as per the
> > >> current changes on drm-intel-nightly.
> > >> Review comments from Ville (12th June 2017) have been addressed
> > >> Review comments from Clinton A Taylor (7th July 2017) have been
> > >> addressed Review comments from Clinton A Taylor (10th July 2017)
> > >> have been addressed. Had missed out tested-by/reviewed-by in the
> > >> patches.
> > >> Fixed that error in this series.
> > >> Review comments from Ville (11th July 2017) addressed.
> > >> Review comments from Paauwe, Bob (29th July 2017) addressed.
> > >>
> > >> Update from last rev:
> > >> Rebased the series as Ville's patches are merged. Previously, this
> > >> series included those floating patches.
> > >>
> > >> Chandra Konduru (6):
> > >>   drm/i915: Set scaler mode for NV12
> > >>   drm/i915: Update format_is_yuv() to include NV12
> > >>   drm/i915: Upscale scaler max scale for NV12
> > >>   drm/i915: Add NV12 as supported format for primary plane
> > >>   drm/i915: Add NV12 as supported format for sprite plane
> > >>   drm/i915: Add NV12 support to intel_framebuffer_init
> > >
> > > Needs serious work on the plane scaling igt (it's atm all broken,
> > > and doesn't test any atomic interactions).
> > >
> > > Then this needs serious work on the nv12 plane igts (which don't yet
> > > exist).
> > >
> > > Then this probably needs pile more igts to test interactions between
> > > everything (e.g. rotation, ...).
> > >
> > > In short: This needs itgs. Lots of them :-)
> > >
> > > Before those exist, and before we've tracked down the bug in the
> > > existing code you're building on it imo makes no sense to start
> > > reviewing these here.
> >
> > Also, this series need to advertise which modifiers work with the new
> > NV12 format by adding a case to
> > skl_sprite_plane_format_mod_supported() and skl_mod_supported().
> 

I am trying to cover as many test cases as possible. Thank you.

> Oh right, which also means we need igts to test the plane formats against
> more modifiers! Well that's kinda part of the tiling tests I guess, at least for
> nv12.

In the current igt test that I have been enhancing, I have added tests for
Tiling and nv12 combination. I have also added sprite and nv12 combination.
Will try to float them as soon as possible. Facing slight issues for larger
Resolution panels (4K) (maybe related to WM etc). Once I test them,
Will float the IGT. Thank you.

> -Daniel
> >
> > Kristian
> >
> > > Thanks, Daniel
> > >
> > >>
> > >>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
> > >>  drivers/gpu/drm/i915/intel_atomic.c  |  8 ++++-
> > >> drivers/gpu/drm/i915/intel_display.c | 67
> +++++++++++++++++++++++++++++-------
> > >>  drivers/gpu/drm/i915/intel_drv.h     |  3 +-
> > >>  drivers/gpu/drm/i915/intel_sprite.c  | 34 ++++++++++++++----
> > >>  5 files changed, 92 insertions(+), 21 deletions(-)
> > >>
> > >> --
> > >> 1.9.1
> > >>
> > >> _______________________________________________
> > >> Intel-gfx mailing list
> > >> Intel-gfx@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation http://blog.ffwll.ch
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx