[PATCH 00/15] drm/i915: Adding NV12 for skylake display

[PATCH 00/15] drm/i915: Adding NV12 for skylake display

[Chandra Konduru]

This patch series is adding initial NV12 support for Skylake display
after rebasing on latest drm-intel-nightly. Earlier I had two patch
series one for 0/180 and another for 90/270. Some of the patches
were already merged. This is combined series to support 0/90/180/270
and removing the ones that are already merged.

Feature is tested with igt/kms_nv12 testcases.
Feature is unit tested for linear/X/Y formats in 0, 90, 180, 270
orientations with combinations of 1 or 2 planes enabled along with
scaling. Also negatively tested for enabling NV12 on unsupported
plane.

The last patch in this series depends on Tvrtko's GEM remapping
for NV12 format patch series.

First two patches fixing couple things in dbuf logic to allocate
correct min number of dbuf blocks and use correct source width
and height in 90/270 rotation cases.

Update from last rev:
-Resolved Ville's full review comments.
-Changes were described in individual patch headers.

So far R-B tag was issued for 1, 4, 6, 9 (in old series)
That changes to 1, 3, 5, 8 because older 3rd patch was squashed.

As number of patches are changed from previous rev due to some
code refactoring, squashing and various minor updates, sending
full series to keep patch numbering consistent.

Chandra Konduru (15):
  drm/i915: Allocate min dbuf blocks per bspec
  drm/i915: In DBUF/WM calcs for 90/270, swap w & h
  drm/i915: Set scaler mode for NV12
  drm/i915: Stage scaler request for NV12 as src format
  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
  drm/i915: Add NV12 to primary plane programming.
  drm/i915: Add NV12 to sprite plane programming.
  drm/i915: Set initial phase & trip for NV12 scaler
  drm/i915: skl nv12 wa - disable streamer fix
  drm/i915: skl nv12 wa - NV12 to RGB switch
  drm/i915: Add 90/270 rotation for NV12 format.

 drivers/gpu/drm/i915/i915_drv.c      |   43 +++++++
 drivers/gpu/drm/i915/i915_reg.h      |   46 +++++++
 drivers/gpu/drm/i915/intel_atomic.c  |    5 +-
 drivers/gpu/drm/i915/intel_csr.c     |   29 -----
 drivers/gpu/drm/i915/intel_display.c |  226 +++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h     |    6 +-
 drivers/gpu/drm/i915/intel_pm.c      |   57 ++++++++-
 drivers/gpu/drm/i915/intel_sprite.c  |  127 +++++++++++++++----
 8 files changed, 464 insertions(+), 75 deletions(-)

-- 
1.7.9.5

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

[PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec

[Chandra Konduru]

Properly allocate min blocks per hw requirements.

v2:
- changed helper functional param to bool, some code simplification (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fff0c22..4d3aca0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2959,6 +2959,31 @@ skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
 	return total_data_rate;
 }
 
+static uint16_t
+skl_dbuf_min_alloc(const struct intel_plane_wm_parameters *p, bool y_plane)
+{
+	uint16_t min_alloc;
+
+	/* For packed formats, no y-plane, return 0 */
+	if (y_plane && !p->y_bytes_per_pixel)
+		return 0;
+
+	if (p->tiling == I915_FORMAT_MOD_Y_TILED ||
+	    p->tiling == I915_FORMAT_MOD_Yf_TILED) {
+		uint32_t min_scanlines = 8;
+		uint8_t bytes_per_pixel =
+			y_plane ? p->y_bytes_per_pixel : p->bytes_per_pixel;
+
+		min_scanlines = 32 / bytes_per_pixel;
+		min_alloc = DIV_ROUND_UP((4 * p->horiz_pixels/(y_plane ? 1 : 2) *
+			bytes_per_pixel), 512) * min_scanlines/4 + 3;
+	} else {
+		min_alloc = 8;
+	}
+
+	return min_alloc;
+}
+
 static void
 skl_allocate_pipe_ddb(struct drm_crtc *crtc,
 		      const struct intel_wm_config *config,
@@ -2999,9 +3024,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
 		if (!p->enabled)
 			continue;
 
-		minimum[plane] = 8;
+		minimum[plane] = skl_dbuf_min_alloc(p, false);   /* uv-plane/packed */
 		alloc_size -= minimum[plane];
-		y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
+		y_minimum[plane] = skl_dbuf_min_alloc(p, true);  /* y-plane */
 		alloc_size -= y_minimum[plane];
 	}
 
-- 
1.7.9.5

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

[PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h

[Chandra Konduru]

This patch swaps src width and height for dbuf/wm calculations
when rotation is 90/270 as per hw requirements.

v2:
- minor/cosmetic changes, removed plane_state check kludge (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |   28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4d3aca0..8a36ab9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3187,10 +3187,14 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 
 	p->active = intel_crtc->active;
 	if (p->active) {
+		const struct intel_plane_state *plane_state;
+		int src_w, src_h;
+
 		p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
 		p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
 
 		fb = crtc->primary->state->fb;
+		plane_state = to_intel_plane_state(crtc->primary->state);
 		/* For planar: Bpp is for uv plane, y_Bpp is for y plane */
 		if (fb) {
 			p->plane[0].enabled = true;
@@ -3205,8 +3209,17 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
 			p->plane[0].y_bytes_per_pixel = 0;
 			p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
 		}
-		p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
-		p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
+
+		src_w = drm_rect_width(&plane_state->src) >> 16;
+		src_h = drm_rect_height(&plane_state->src) >> 16;
+
+		if (intel_rotation_90_or_270(crtc->primary->state->rotation)) {
+			p->plane[0].horiz_pixels = src_h;
+			p->plane[0].vert_pixels = src_w;
+		} else {
+			p->plane[0].horiz_pixels = src_w;
+			p->plane[0].vert_pixels = src_h;
+		}
 		p->plane[0].rotation = crtc->primary->state->rotation;
 
 		fb = crtc->cursor->state->fb;
@@ -3740,8 +3753,15 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	intel_plane->wm.enabled = enabled;
 	intel_plane->wm.scaled = scaled;
-	intel_plane->wm.horiz_pixels = sprite_width;
-	intel_plane->wm.vert_pixels = sprite_height;
+
+	if (intel_rotation_90_or_270(plane->state->rotation)) {
+		intel_plane->wm.horiz_pixels = sprite_height;
+		intel_plane->wm.vert_pixels = sprite_width;
+	} else {
+		intel_plane->wm.horiz_pixels = sprite_width;
+		intel_plane->wm.vert_pixels = sprite_height;
+	}
+
 	intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
 
 	/* For planar: Bpp is for UV plane, y_Bpp is for Y plane */
-- 
1.7.9.5

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

[PATCH 03/15] drm/i915: Set scaler mode for NV12

[Chandra Konduru]

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:
- new reg defines squashed into patches used them (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h     |    1 +
 drivers/gpu/drm/i915/intel_atomic.c |    5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1fa0554..825d721 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5498,6 +5498,7 @@ enum skl_disp_power_wells {
 #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 9336e80..fd3972c 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -247,7 +247,10 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
 		}
 
 		/* set scaler mode */
-		if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
+		if (plane_state && plane_state->base.fb &&
+			plane_state->base.fb->pixel_format == DRM_FORMAT_NV12) {
+			scaler_state->scalers[*scaler_id].mode = PS_SCALER_MODE_NV12;
+		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
 			/*
 			 * when only 1 scaler is in use on either pipe A or B,
 			 * scaler 0 operates in high quality (HQ) mode.
-- 
1.7.9.5

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

[PATCH 04/15] drm/i915: Stage scaler request for NV12 as src format

[Chandra Konduru]

This patch stages a scaler request when input format
is NV12. The same scaler does both chroma-upsampling
and resolution scaling as needed.

v2:
-Added helper function for need_scaling (Ville)

v3:
-Rebased to current kernel version 4.2.0.rc4 (me)

v4:
-minor updates (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3ee1c17..8869779 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4341,20 +4341,38 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 	}
 }
 
+static bool skl_need_scaling(int src_w, int src_h, int dst_w, int dst_h,
+	unsigned int rotation, uint32_t pixel_format)
+{
+	/* need a scaler when sizes doesn't match */
+	if (src_w != dst_w || src_h != dst_h)
+		return true;
+
+	/* in case of 90/270 rotation, check src width with dst height and so */
+	if (intel_rotation_90_or_270(rotation) &&
+			(src_h != dst_w || src_w != dst_h))
+		return true;
+
+	/* need a scaler for nv12 */
+	if (pixel_format == DRM_FORMAT_NV12)
+		return true;
+
+	return false;
+}
+
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		  unsigned scaler_user, int *scaler_id, unsigned int rotation,
-		  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;
 	struct intel_crtc *intel_crtc =
 		to_intel_crtc(crtc_state->base.crtc);
-	int need_scaling;
+	bool need_scaling;
 
-	need_scaling = intel_rotation_90_or_270(rotation) ?
-		(src_h != dst_w || src_w != dst_h):
-		(src_w != dst_w || src_h != dst_h);
+	need_scaling = skl_need_scaling(src_w, src_h, dst_w, dst_h, rotation,
+		pixel_format);
 
 	/*
 	 * if plane is being disabled or scaler is no more required or force detach
@@ -4423,7 +4441,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, DRM_ROTATE_0,
 		state->pipe_src_w, state->pipe_src_h,
-		adjusted_mode->hdisplay, adjusted_mode->vdisplay);
+		adjusted_mode->hdisplay, adjusted_mode->vdisplay, 0);
 }
 
 /**
@@ -4459,7 +4477,8 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 				drm_rect_width(&plane_state->src) >> 16,
 				drm_rect_height(&plane_state->src) >> 16,
 				drm_rect_width(&plane_state->dst),
-				drm_rect_height(&plane_state->dst));
+				drm_rect_height(&plane_state->dst),
+				fb ? fb->pixel_format : 0);
 
 	if (ret || plane_state->scaler_id < 0)
 		return ret;
@@ -4484,6 +4503,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] FB:%d unsupported scaling format 0x%x\n",
-- 
1.7.9.5

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

[PATCH 05/15] drm/i915: Update format_is_yuv() to include NV12

[Chandra Konduru]

This patch adds NV12 to format_is_yuv() function
and made it available for both primary and sprite
planes.

v2:
-Use intel_ prefix for format_is_yuv (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |    1 +
 drivers/gpu/drm/i915/intel_sprite.c |    9 +++++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f44941b..18632a4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1394,6 +1394,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 void intel_pipe_update_start(struct intel_crtc *crtc,
 			     uint32_t *start_vbl_count);
 void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count);
+bool intel_format_is_yuv(uint32_t format);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c13c529..8b73bb8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -39,14 +39,15 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
-static bool
-format_is_yuv(uint32_t format)
+bool
+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;
@@ -293,7 +294,7 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
 	int plane = intel_plane->plane;
 
 	/* Seems RGB data bypasses the CSC always */
-	if (!format_is_yuv(format))
+	if (!intel_format_is_yuv(format))
 		return;
 
 	/*
@@ -857,7 +858,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 		src_y = src->y1 >> 16;
 		src_h = drm_rect_height(src) >> 16;
 
-		if (format_is_yuv(fb->pixel_format)) {
+		if (intel_format_is_yuv(fb->pixel_format)) {
 			src_x &= ~1;
 			src_w &= ~1;
 
-- 
1.7.9.5

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

[PATCH 06/15] drm/i915: Upscale scaler max scale for NV12.

[Chandra Konduru]

This patch updates max supported scaler limits for NV12.

v2:
-Rebased to current kernel version 4.2.0.rc4 (me)

v3:
-simplified max_scale calculation (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   17 +++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h     |    3 ++-
 drivers/gpu/drm/i915/intel_sprite.c  |    2 +-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8869779..3f9111e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13423,7 +13423,9 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 }
 
 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)
 {
 	int max_scale;
 	struct drm_device *dev;
@@ -13443,11 +13445,17 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
 
 	/*
 	 * skl max scale is lower of:
-	 *    close to 3 but not 3, -1 is for that purpose
+	 *    close to 2 or 3 (NV12: 2, other formats: 3) but not equal,
+	 *          -1 is for that purpose
 	 *            or
 	 *    cdclk/crtc_clock
 	 */
-	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
+	if (pixel_format == DRM_FORMAT_NV12)
+		max_scale = (2 << 16) - 1;
+	else
+		max_scale = (3 << 16) - 1;
+
+	max_scale = min(max_scale, (1 << 8) * ((cdclk << 8) / crtc_clock));
 
 	return max_scale;
 }
@@ -13467,7 +13475,8 @@ intel_check_primary_plane(struct drm_plane *plane,
 	if (INTEL_INFO(plane->dev)->gen >= 9 &&
 	    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,
+				fb ? fb->pixel_format : 0);
 		can_position = true;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 18632a4..d50b8cb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1140,7 +1140,8 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
 void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
 
 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);
 
 unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
 				     struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 8b73bb8..66d60ae 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -780,7 +780,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 		if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
 			can_scale = 1;
 			min_scale = 1;
-			max_scale = skl_max_scale(intel_crtc, crtc_state);
+			max_scale = skl_max_scale(intel_crtc, crtc_state, fb->pixel_format);
 		} else {
 			can_scale = 0;
 			min_scale = DRM_PLANE_HELPER_NO_SCALING;
-- 
1.7.9.5

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

[PATCH 07/15] drm/i915: Add NV12 as supported format for primary plane

[Chandra Konduru]

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

v2:
-Rebased (me)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_display.c |   22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3f9111e..84dad95 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -74,6 +74,19 @@ static const uint32_t skl_primary_formats[] = {
 	DRM_FORMAT_XBGR2101010,
 };
 
+/* Primary plane formats for gen >= 9 with NV12 */
+static const uint32_t skl_primary_formats_with_nv12[] = {
+	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_NV12,
+};
+
 /* Cursor formats */
 static const uint32_t intel_cursor_formats[] = {
 	DRM_FORMAT_ARGB8888,
@@ -13612,8 +13625,13 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 		primary->plane = !pipe;
 
 	if (INTEL_INFO(dev)->gen >= 9) {
-		intel_primary_formats = skl_primary_formats;
-		num_formats = ARRAY_SIZE(skl_primary_formats);
+		if (pipe == PIPE_A || pipe == PIPE_B) {
+			intel_primary_formats = skl_primary_formats_with_nv12;
+			num_formats = ARRAY_SIZE(skl_primary_formats_with_nv12);
+		} else {
+			intel_primary_formats = skl_primary_formats;
+			num_formats = ARRAY_SIZE(skl_primary_formats);
+		}
 	} else if (INTEL_INFO(dev)->gen >= 4) {
 		intel_primary_formats = i965_primary_formats;
 		num_formats = ARRAY_SIZE(i965_primary_formats);
-- 
1.7.9.5

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

[PATCH 08/15] drm/i915: Add NV12 as supported format for sprite plane

[Chandra Konduru]

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

v2:
- made supported format list const, fixed a leftover -1. (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_sprite.c |   25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 66d60ae..797594e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1029,7 +1029,7 @@ static const uint32_t vlv_plane_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
-static uint32_t skl_plane_formats[] = {
+static const uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_ARGB8888,
@@ -1041,6 +1041,19 @@ static uint32_t skl_plane_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
+static const uint32_t skl_plane_formats_with_nv12[] = {
+	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,
+};
+
 int
 intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 {
@@ -1112,8 +1125,14 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 		intel_plane->disable_plane = skl_disable_plane;
 		state->scaler_id = -1;
 
-		plane_formats = skl_plane_formats;
-		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		if ((pipe == PIPE_A || pipe == PIPE_B) && (plane == 0)) {
+			plane_formats = skl_plane_formats_with_nv12;
+			num_plane_formats = ARRAY_SIZE(skl_plane_formats_with_nv12);
+		} else {
+			plane_formats = skl_plane_formats;
+			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
+		}
+
 		break;
 	default:
 		kfree(intel_plane);
-- 
1.7.9.5

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

[PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Chandra Konduru]

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

v2:
-Fix an issue in checks added (me)

v3:
-cosmetic update, split checks into two (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_display.c |   33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 84dad95..5433c6d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14349,9 +14349,40 @@ static int intel_framebuffer_init(struct drm_device *dev,
 			return -EINVAL;
 		}
 		break;
+	case DRM_FORMAT_NV12:
+		if (INTEL_INFO(dev)->gen < 9) {
+			DRM_DEBUG("unsupported pixel format: %s\n",
+				drm_get_format_name(mode_cmd->pixel_format));
+			return -EINVAL;
+		}
+		if (!mode_cmd->offsets[1]) {
+			DRM_DEBUG("uv start offset not set\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
+			DRM_DEBUG("y and uv subplanes have different pitches\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->handles[0] != mode_cmd->handles[1]) {
+			DRM_DEBUG("y and uv subplanes have different handles\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
+			(mode_cmd->offsets[1] & 0xFFF)) {
+			DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new tile-row\n",
+				mode_cmd->offsets[1]);
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
+			((mode_cmd->offsets[1] / mode_cmd->pitches[1]) % 4)) {
+			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line aligned\n",
+				mode_cmd->offsets[1]);
+			return -EINVAL;
+		}
+		break;
 	default:
 		DRM_DEBUG("unsupported pixel format: %s\n",
-			  drm_get_format_name(mode_cmd->pixel_format));
+			drm_get_format_name(mode_cmd->pixel_format));
 		return -EINVAL;
 	}
 
-- 
1.7.9.5

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

[PATCH 10/15] drm/i915: Add NV12 to primary plane programming.

[Chandra Konduru]

This patch is adding NV12 support to skylake primary plane
programming. It is covering linear/X/Y/Yf tiling formats
for 0 and 180 rotations.

For 90/270 rotation, Y and UV subplanes should be treated
as separate surfaces and GTT remapping for rotation should
be done separately for each subplane. Once GEM adds support
for seperate remappings for two subplanes, 90/270 support
to be added to plane programming.

v2:
-Use regular int instead of 16.16 in aux_offset calculations (me)

v3:
-Allow 90/270 for NV12 as its remapping is now supported (me)

v4:
-Rebased to current kernel version 4.2.0.rc4 (me)

v5:
-Used round up division for aux_stride calculation,
 adjusted dst rect as part of macro pixel boundary calculation (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/i915_reg.h      |   25 ++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |   59 ++++++++++++++++++++++++++++++++--
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 825d721..ae40b22 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5602,6 +5602,31 @@ enum skl_disp_power_wells {
 			_ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A),   \
 			_ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B)
 
+/*
+ * Skylake  NV12 Register
+ */
+#define PLANE_AUX_DIST_1_A		0x701c0
+#define PLANE_AUX_DIST_2_A		0x702c0
+#define PLANE_AUX_DIST_1_B		0x711c0
+#define PLANE_AUX_DIST_2_B		0x712c0
+#define _PLANE_AUX_DIST_1(pipe)	\
+   _PIPE(pipe, PLANE_AUX_DIST_1_A, PLANE_AUX_DIST_1_B)
+#define _PLANE_AUX_DIST_2(pipe)	\
+   _PIPE(pipe, PLANE_AUX_DIST_2_A, PLANE_AUX_DIST_2_B)
+#define PLANE_AUX_DIST(pipe, plane)	\
+	_PLANE(plane, _PLANE_AUX_DIST_1(pipe), _PLANE_AUX_DIST_2(pipe))
+
+#define PLANE_AUX_OFFSET_1_A		0x701c4
+#define PLANE_AUX_OFFSET_2_A		0x702c4
+#define PLANE_AUX_OFFSET_1_B		0x711c4
+#define PLANE_AUX_OFFSET_2_B		0x712c4
+#define _PLANE_AUX_OFFSET_1(pipe)	\
+   _PIPE(pipe, PLANE_AUX_OFFSET_1_A, PLANE_AUX_OFFSET_1_B)
+#define _PLANE_AUX_OFFSET_2(pipe)	\
+   _PIPE(pipe, PLANE_AUX_OFFSET_2_A, PLANE_AUX_OFFSET_2_B)
+#define PLANE_AUX_OFFSET(pipe, plane)	\
+	_PLANE(plane, _PLANE_AUX_OFFSET_1(pipe), _PLANE_AUX_OFFSET_2(pipe))
+
 /* legacy palette */
 #define _LGC_PALETTE_A           0x4a000
 #define _LGC_PALETTE_B           0x4a800
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5433c6d..6714066 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3026,6 +3026,8 @@ 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);
 	}
@@ -3094,6 +3096,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
 	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
 	int scaler_id = -1;
+	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
+	u32 tile_row_adjustment = 0;
 
 	plane_state = to_intel_plane_state(plane->state);
 
@@ -3150,11 +3154,34 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 		x_offset = stride * tile_height - y - src_h;
 		y_offset = x;
 		plane_size = (src_w - 1) << 16 | (src_h - 1);
+		/*
+		 * TBD: For NV12 90/270 rotation, Y and UV subplanes should
+		 * be treated as separate surfaces and GTT remapping for
+		 * rotation should be done separately for each subplane.
+		 * Enable support once seperate remappings are available.
+		 */
 	} else {
 		stride = fb->pitches[0] / stride_div;
 		x_offset = x;
 		y_offset = y;
 		plane_size = (src_h - 1) << 16 | (src_w - 1);
+		tile_height = PAGE_SIZE / stride_div;
+
+		if (fb->pixel_format == DRM_FORMAT_NV12) {
+			int height_in_mem = (fb->offsets[1]/fb->pitches[0]);
+			/*
+			 * If UV starts from middle of a page, then UV start should
+			 * be programmed to beginning of that page. And offset into that
+			 * page to be programmed into y-offset
+			 */
+			tile_row_adjustment = height_in_mem % tile_height;
+			aux_dist = fb->pitches[0] * (height_in_mem - tile_row_adjustment);
+			aux_x_offset = DIV_ROUND_UP(x, 2);
+			aux_y_offset = DIV_ROUND_UP(y, 2) + tile_row_adjustment;
+			/* For tile-Yf, uv-subplane tile width is 2x of Y-subplane */
+			aux_stride = fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED ?
+				DIV_ROUND_UP(stride, 2) : stride;
+		}
 	}
 	plane_offset = y_offset << 16 | x_offset;
 
@@ -3162,11 +3189,14 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
 	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
 	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
+	I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
+	I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset << 16 | aux_x_offset);
 
 	if (scaler_id >= 0) {
 		uint32_t ps_ctrl = 0;
 
 		WARN_ON(!dst_w || !dst_h);
+
 		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
 			crtc_state->scaler_state.scalers[scaler_id].mode;
 		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
@@ -3175,6 +3205,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w << 16) | dst_h);
 		I915_WRITE(PLANE_POS(pipe, 0), 0);
 	} else {
+		WARN_ON(fb->pixel_format == DRM_FORMAT_NV12);
 		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
 	}
 
@@ -11627,12 +11658,36 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 
 	bool turn_off, turn_on, visible, was_visible;
 	struct drm_framebuffer *fb = plane_state->fb;
+	struct intel_plane_state *intel_plane_state =
+			to_intel_plane_state(plane_state);
+
+	/* Adjust (macro)pixel boundary */
+	if (fb && intel_format_is_yuv(fb->pixel_format)) {
+		bool can_scale = false;
+
+		intel_plane_state->src.x1 &= ~0x10000;
+		intel_plane_state->src.x2 &= ~0x10000;
+
+		/* scaler use allowed when colorkey isn't requested */
+		if ((INTEL_INFO(dev)->gen >= 9) &&
+			(intel_plane_state->ckey.flags == I915_SET_COLORKEY_NONE))
+				can_scale = true;
+
+		/* Must keep src and dst the same if we can't scale. */
+		if (!can_scale) {
+			intel_plane_state->dst.x1 &= ~1;
+			intel_plane_state->dst.x2 &= ~1;
+		}
+
+		if (drm_rect_width(&intel_plane_state->dst) == 0)
+			intel_plane_state->visible = false;
+	}
 
 	if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
 	    plane->type != DRM_PLANE_TYPE_CURSOR) {
 		ret = skl_update_scaler_plane(
 			to_intel_crtc_state(crtc_state),
-			to_intel_plane_state(plane_state));
+			intel_plane_state);
 		if (ret)
 			return ret;
 	}
@@ -11646,7 +11701,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 		intel_crtc->atomic.disabled_planes |= 1 << i;
 
 	was_visible = old_plane_state->visible;
-	visible = to_intel_plane_state(plane_state)->visible;
+	visible = intel_plane_state->visible;
 
 	if (!was_crtc_enabled && WARN_ON(was_visible))
 		was_visible = false;
-- 
1.7.9.5

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

[PATCH 11/15] drm/i915: Add NV12 to sprite plane programming.

[Chandra Konduru]

This patch is adding NV12 support to skylake sprite plane
programming. It is covering linear/X/Y/Yf tiling formats
for 0 and 180 rotations.

For 90/270 rotation, Y and UV subplanes should be treated
as separate surfaces and GTT remapping for rotation should
be done separately for each subplane. Once GEM adds support
for seperate remappings for two subplanes, 90/270 support
to be added to plane programming.

v2:
-Use round up division for aux_stride calculation (me)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_sprite.c |   31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 797594e..347fb1f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -188,6 +188,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	int x_offset, y_offset;
 	struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
 	int scaler_id;
+	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
+	u32 tile_row_adjustment = 0;
 
 	plane_ctl = PLANE_CTL_ENABLE |
 		PLANE_CTL_PIPE_CSC_ENABLE;
@@ -234,24 +236,48 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 		plane_size = (src_w << 16) | src_h;
 		x_offset = stride * tile_height - y - (src_h + 1);
 		y_offset = x;
+
+		/*
+		 * TBD: For NV12 90/270 rotation, Y and UV subplanes should
+		 * be treated as separate surfaces and GTT remapping for
+		 * rotation should be done separately for each subplane.
+		 * Enable support once seperate remappings are available.
+		 */
 	} else {
 		stride = fb->pitches[0] / stride_div;
 		plane_size = (src_h << 16) | src_w;
 		x_offset = x;
 		y_offset = y;
+		tile_height = PAGE_SIZE / stride_div;
+
+		if (fb->pixel_format == DRM_FORMAT_NV12) {
+			int height_in_mem = (fb->offsets[1]/fb->pitches[0]);
+			/*
+			 * If UV starts from middle of a page, then UV start should
+			 * be programmed to beginning of that page. And offset into that
+			 * page to be programmed into y-offset
+			 */
+			tile_row_adjustment = height_in_mem % tile_height;
+			aux_dist = fb->pitches[0] * (height_in_mem - tile_row_adjustment);
+			aux_x_offset = DIV_ROUND_UP(x, 2);
+			aux_y_offset = DIV_ROUND_UP(y, 2) + tile_row_adjustment;
+			/* For tile-Yf, uv-subplane tile width is 2x of Y-subplane */
+			aux_stride = fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED ?
+				DIV_ROUND_UP(stride, 2) : stride;
+		}
 	}
 	plane_offset = y_offset << 16 | x_offset;
 
 	I915_WRITE(PLANE_OFFSET(pipe, plane), plane_offset);
 	I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
 	I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
+	I915_WRITE(PLANE_AUX_DIST(pipe, plane), aux_dist | aux_stride);
+	I915_WRITE(PLANE_AUX_OFFSET(pipe, plane), aux_y_offset<<16 | aux_x_offset);
 
 	/* program plane scaler */
 	if (scaler_id >= 0) {
 		uint32_t ps_ctrl = 0;
 
-		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
-			PS_PLANE_SEL(plane));
 		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) |
 			crtc_state->scaler_state.scalers[scaler_id].mode;
 		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
@@ -262,6 +288,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 
 		I915_WRITE(PLANE_POS(pipe, plane), 0);
 	} else {
+		WARN_ON(fb->pixel_format == DRM_FORMAT_NV12);
 		I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
 	}
 
-- 
1.7.9.5

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

[PATCH 12/15] drm/i915: Set initial phase & trip for NV12 scaler

[Chandra Konduru]

This patch sets default initial phase and trip to scale NV12
content. In future, if needed these can be set via properties
or other means depending on incoming stream request. Until then
defaults are fine.

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |    7 +++++++
 drivers/gpu/drm/i915/intel_sprite.c  |    7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6714066..3296d16 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3098,6 +3098,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	int scaler_id = -1;
 	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
 	u32 tile_row_adjustment = 0;
+	u32 hphase = 0, vphase = 0;
 
 	plane_state = to_intel_plane_state(plane->state);
 
@@ -3181,6 +3182,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 			/* For tile-Yf, uv-subplane tile width is 2x of Y-subplane */
 			aux_stride = fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED ?
 				DIV_ROUND_UP(stride, 2) : stride;
+
+			hphase = 0x00010001;  /* use trip for both Y and UV */
+			vphase = 0x00012000;  /* use trip for Y and phase 0.5 for UV */
 		}
 	}
 	plane_offset = y_offset << 16 | x_offset;
@@ -3209,6 +3213,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
 	}
 
+	I915_WRITE(SKL_PS_HPHASE(pipe, scaler_id), hphase);
+	I915_WRITE(SKL_PS_VPHASE(pipe, scaler_id), vphase);
+
 	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
 
 	POSTING_READ(PLANE_SURF(pipe, 0));
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 347fb1f..5ca62b6 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -190,6 +190,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	int scaler_id;
 	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
 	u32 tile_row_adjustment = 0;
+	u32 hphase = 0, vphase = 0;
 
 	plane_ctl = PLANE_CTL_ENABLE |
 		PLANE_CTL_PIPE_CSC_ENABLE;
@@ -264,6 +265,9 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 			/* For tile-Yf, uv-subplane tile width is 2x of Y-subplane */
 			aux_stride = fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED ?
 				DIV_ROUND_UP(stride, 2) : stride;
+
+			hphase = 0x00010001;  /* use trip for both Y and UV */
+			vphase = 0x00012000;  /* use trip for Y and phase 0.5 for UV */
 		}
 	}
 	plane_offset = y_offset << 16 | x_offset;
@@ -292,6 +296,9 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 		I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
 	}
 
+	I915_WRITE(SKL_PS_HPHASE(pipe, scaler_id), hphase);
+	I915_WRITE(SKL_PS_VPHASE(pipe, scaler_id), vphase);
+
 	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
 	I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
 	POSTING_READ(PLANE_SURF(pipe, plane));
-- 
1.7.9.5

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

[PATCH 13/15] drm/i915: skl nv12 wa - disable streamer fix

[Chandra Konduru]

When the plane source pixel format is NV12, the CHICKEN_PIPESL
register bit 22 must be set to 1

v2:
-one wa per commit with comments, and function headers (Daniel)

v3:
-moved intel stepping helper functions to i915_drv.c (Daniel)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |   43 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h      |   12 ++++++++++
 drivers/gpu/drm/i915/intel_csr.c     |   29 -----------------------
 drivers/gpu/drm/i915/intel_display.c |   11 +++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    2 ++
 drivers/gpu/drm/i915/intel_sprite.c  |   11 +++++++++
 6 files changed, 79 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1d88745..0006369 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -391,6 +391,49 @@ static const struct intel_device_info intel_broxton_info = {
 	IVB_CURSOR_OFFSETS,
 };
 
+/* stepping info */
+struct stepping_info {
+	char stepping;
+	char substepping;
+};
+
+/* skl stepping info */
+static const struct stepping_info skl_stepping_info[] = {
+	{'A', '0'}, {'B', '0'}, {'C', '0'},
+	{'D', '0'}, {'E', '0'}, {'F', '0'},
+	{'G', '0'}, {'H', '0'}, {'I', '0'},
+};
+
+/**
+ * intel_get_stepping() - get stepping info
+ * @dev: drm device.
+ *
+ * Returns stepping id 'A', 'B', 'C', etc.
+ */
+char intel_get_stepping(struct drm_device *dev)
+{
+	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
+			ARRAY_SIZE(skl_stepping_info)))
+		return skl_stepping_info[dev->pdev->revision].stepping;
+	else
+		return -ENODATA;
+}
+
+/**
+ * intel_get_substepping() - get substepping info
+ * @dev: drm device.
+ *
+ * Returns substepping id '0', '1', '2', etc.
+ */
+char intel_get_substepping(struct drm_device *dev)
+{
+	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
+			ARRAY_SIZE(skl_stepping_info)))
+		return skl_stepping_info[dev->pdev->revision].substepping;
+	else
+		return -ENODATA;
+}
+
 /*
  * Make sure any device matches here are from most specific to most
  * general.  For example, since the Quanta match is based on the subsystem
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ae40b22..d20f235 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5354,6 +5354,18 @@ enum skl_disp_power_wells {
 #define PLANE_NV12_BUF_CFG(pipe, plane)	\
 	_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), _PLANE_NV12_BUF_CFG_2(pipe))
 
+/*
+ * Skylake Chicken registers
+ */
+#define _CHICKEN_PIPESL_A          0x420B0
+#define _CHICKEN_PIPESL_B          0x420B4
+#define _CHICKEN_PIPESL_C          0x420B8
+#define  DISABLE_STREAMER_FIX      (1 << 22)
+#define CHICKEN_PIPESL(pipe) _PIPE(pipe, _CHICKEN_PIPESL_A, _CHICKEN_PIPESL_B)
+
+#define CHICKEN_DCPR_1             0x46430
+#define IDLE_WAKEMEM_MASK          (1 << 13)
+
 /* SKL new cursor registers */
 #define _CUR_BUF_CFG_A				0x7017c
 #define _CUR_BUF_CFG_B				0x7117c
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index ba1ae03..9577727 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -170,35 +170,6 @@ struct intel_dmc_header {
 	uint32_t reserved1[2];
 } __packed;
 
-struct stepping_info {
-	char stepping;
-	char substepping;
-};
-
-static const struct stepping_info skl_stepping_info[] = {
-		{'A', '0'}, {'B', '0'}, {'C', '0'},
-		{'D', '0'}, {'E', '0'}, {'F', '0'},
-		{'G', '0'}, {'H', '0'}, {'I', '0'}
-};
-
-static char intel_get_stepping(struct drm_device *dev)
-{
-	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
-			ARRAY_SIZE(skl_stepping_info)))
-		return skl_stepping_info[dev->pdev->revision].stepping;
-	else
-		return -ENODATA;
-}
-
-static char intel_get_substepping(struct drm_device *dev)
-{
-	if (IS_SKYLAKE(dev) && (dev->pdev->revision <
-			ARRAY_SIZE(skl_stepping_info)))
-		return skl_stepping_info[dev->pdev->revision].substepping;
-	else
-		return -ENODATA;
-}
-
 /**
  * intel_csr_load_status_get() - to get firmware loading status.
  * @dev_priv: i915 device.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3296d16..9e11439 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3196,6 +3196,17 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	I915_WRITE(PLANE_AUX_DIST(pipe, 0), aux_dist | aux_stride);
 	I915_WRITE(PLANE_AUX_OFFSET(pipe, 0), aux_y_offset << 16 | aux_x_offset);
 
+	/*
+	 * Per bspec, for SKL C and BXT A steppings, when the plane source pixel
+	 * format is NV12, the CHICKEN_PIPESL register bit 22 must be set to 1
+	 */
+	if (((IS_SKYLAKE(dev) && intel_get_stepping(dev) == 'C') ||
+		(IS_BROXTON(dev) && intel_get_stepping(dev) == 'A')) &&
+		fb->pixel_format == DRM_FORMAT_NV12) {
+			I915_WRITE(CHICKEN_PIPESL(pipe),
+				I915_READ(CHICKEN_PIPESL(pipe)) | DISABLE_STREAMER_FIX);
+	}
+
 	if (scaler_id >= 0) {
 		uint32_t ps_ctrl = 0;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d50b8cb..59549245 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -992,6 +992,8 @@ void i915_audio_component_cleanup(struct drm_i915_private *dev_priv);
 /* intel_display.c */
 extern const struct drm_plane_funcs intel_plane_funcs;
 bool intel_has_pending_fb_unpin(struct drm_device *dev);
+char intel_get_stepping(struct drm_device *dev);
+char intel_get_substepping(struct drm_device *dev);
 int intel_pch_rawclk(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
 void intel_mark_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 5ca62b6..07f88f3 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -278,6 +278,17 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	I915_WRITE(PLANE_AUX_DIST(pipe, plane), aux_dist | aux_stride);
 	I915_WRITE(PLANE_AUX_OFFSET(pipe, plane), aux_y_offset<<16 | aux_x_offset);
 
+	/*
+	 * Per bspec, for SKL C and BXT A steppings, when the plane source pixel
+	 * format is NV12, the CHICKEN_PIPESL register bit 22 must be set to 1
+	 */
+	if (((IS_SKYLAKE(dev) && intel_get_stepping(dev) == 'C') ||
+		(IS_BROXTON(dev) && intel_get_stepping(dev) == 'A')) &&
+		fb->pixel_format == DRM_FORMAT_NV12) {
+			I915_WRITE(CHICKEN_PIPESL(pipe),
+				I915_READ(CHICKEN_PIPESL(pipe)) | DISABLE_STREAMER_FIX);
+	}
+
 	/* program plane scaler */
 	if (scaler_id >= 0) {
 		uint32_t ps_ctrl = 0;
-- 
1.7.9.5

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

[PATCH 14/15] drm/i915: skl nv12 wa - NV12 to RGB switch

[Chandra Konduru]

Switching format from NV12 to RGB can result in display underrun
and corruption. This workaround sets bits 15 & 19 to 1 in
CLKGATE_DIS_PSL register to address transition underrun.

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |    8 ++++++++
 drivers/gpu/drm/i915/intel_display.c |   25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d20f235..2e2636d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5366,6 +5366,14 @@ enum skl_disp_power_wells {
 #define CHICKEN_DCPR_1             0x46430
 #define IDLE_WAKEMEM_MASK          (1 << 13)
 
+#define CLKGATE_DIS_PSL_A        0x46520
+#define CLKGATE_DIS_PSL_B        0x46524
+#define CLKGATE_DIS_PSL_C        0x46528
+#define DUPS1_GATING_DIS         (1 << 15)
+#define DUPS2_GATING_DIS         (1 << 19)
+#define DUPS3_GATING_DIS         (1 << 23)
+#define CLKGATE_DIS_PSL(pipe)  _PIPE(pipe, CLKGATE_DIS_PSL_A, CLKGATE_DIS_PSL_B)
+
 /* SKL new cursor registers */
 #define _CUR_BUF_CFG_A				0x7017c
 #define _CUR_BUF_CFG_B				0x7117c
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9e11439..457b79b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5017,6 +5017,25 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
 	return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
 }
 
+/*
+ * Switching format from NV12 to RGB can result in display underrun
+ * and corruption. Workaround is to set bits 15 & 19 to 1 in
+ * CLKGATE_DIS_PSL register.
+ */
+static void skl_wa_clkgate(struct drm_i915_private *dev_priv,
+	int pipe, int enable)
+{
+	if (pipe == PIPE_A || pipe == PIPE_B) {
+		if (enable)
+			I915_WRITE(CLKGATE_DIS_PSL(pipe),
+				DUPS1_GATING_DIS | DUPS2_GATING_DIS);
+		else
+			I915_WRITE(CLKGATE_DIS_PSL(pipe),
+				I915_READ(CLKGATE_DIS_PSL(pipe) &
+				~(DUPS1_GATING_DIS|DUPS2_GATING_DIS)));
+	}
+}
+
 static void haswell_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -5107,6 +5126,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 		intel_wait_for_vblank(dev, hsw_workaround_pipe);
 		intel_wait_for_vblank(dev, hsw_workaround_pipe);
 	}
+
+	/* workaround for NV12 */
+	skl_wa_clkgate(dev_priv, pipe, 1);
 }
 
 static void ironlake_pfit_disable(struct intel_crtc *crtc)
@@ -5224,6 +5246,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	intel_crtc->active = false;
 	intel_update_watermarks(crtc);
+
+	/* workaround for NV12 */
+	skl_wa_clkgate(dev_priv, intel_crtc->pipe, 0);
 }
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
-- 
1.7.9.5

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

[PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format.

[Chandra Konduru]

Adding NV12 90/270 rotation support for primary and sprite planes.

v2:
-For 90/270 adjust pixel boundary only in Y-direction (bspec)

v3:
-Rebased (me)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_display.c |   46 +++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_sprite.c  |   56 ++++++++++++++++++++++------------
 2 files changed, 69 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 457b79b..4501e48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3096,7 +3096,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
 	int dst_x = 0, dst_y = 0, dst_w = 0, dst_h = 0;
 	int scaler_id = -1;
-	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
+	unsigned long aux_dist = 0;
+	u32 aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
 	u32 tile_row_adjustment = 0;
 	u32 hphase = 0, vphase = 0;
 
@@ -3155,12 +3156,16 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 		x_offset = stride * tile_height - y - src_h;
 		y_offset = x;
 		plane_size = (src_w - 1) << 16 | (src_h - 1);
-		/*
-		 * TBD: For NV12 90/270 rotation, Y and UV subplanes should
-		 * be treated as separate surfaces and GTT remapping for
-		 * rotation should be done separately for each subplane.
-		 * Enable support once seperate remappings are available.
-		 */
+
+		if (fb->pixel_format == DRM_FORMAT_NV12) {
+			u32 uv_tile_height = intel_tile_height(dev, fb->pixel_format,
+						fb->modifier[0], 1);
+			aux_stride = DIV_ROUND_UP(fb->height / 2, uv_tile_height);
+			aux_dist = intel_plane_obj_offset(to_intel_plane(plane), obj, 1) -
+				surf_addr;
+			aux_x_offset = aux_stride * uv_tile_height - y / 2 - fb->height / 2;
+			aux_y_offset = x / 2;
+		}
 	} else {
 		stride = fb->pitches[0] / stride_div;
 		x_offset = x;
@@ -11708,8 +11713,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	if (fb && intel_format_is_yuv(fb->pixel_format)) {
 		bool can_scale = false;
 
-		intel_plane_state->src.x1 &= ~0x10000;
-		intel_plane_state->src.x2 &= ~0x10000;
+		if (intel_rotation_90_or_270(plane_state->rotation)) {
+			intel_plane_state->src.y1 &= ~0x10000;
+			intel_plane_state->src.y2 &= ~0x10000;
+		} else {
+			intel_plane_state->src.x1 &= ~0x10000;
+			intel_plane_state->src.x2 &= ~0x10000;
+		}
 
 		/* scaler use allowed when colorkey isn't requested */
 		if ((INTEL_INFO(dev)->gen >= 9) &&
@@ -11718,12 +11728,20 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 
 		/* Must keep src and dst the same if we can't scale. */
 		if (!can_scale) {
-			intel_plane_state->dst.x1 &= ~1;
-			intel_plane_state->dst.x2 &= ~1;
+			if (intel_rotation_90_or_270(plane_state->rotation)) {
+				intel_plane_state->dst.y1 &= ~1;
+				intel_plane_state->dst.y2 &= ~1;
+
+				if (drm_rect_height(&intel_plane_state->dst) == 0)
+					intel_plane_state->visible = false;
+			} else {
+				intel_plane_state->dst.x1 &= ~1;
+				intel_plane_state->dst.x2 &= ~1;
+
+				if (drm_rect_width(&intel_plane_state->dst) == 0)
+					intel_plane_state->visible = false;
+			}
 		}
-
-		if (drm_rect_width(&intel_plane_state->dst) == 0)
-			intel_plane_state->visible = false;
 	}
 
 	if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 07f88f3..3cf601d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -188,7 +188,8 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 	int x_offset, y_offset;
 	struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
 	int scaler_id;
-	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
+	unsigned long aux_dist = 0;
+	u32 aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
 	u32 tile_row_adjustment = 0;
 	u32 hphase = 0, vphase = 0;
 
@@ -238,12 +239,14 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 		x_offset = stride * tile_height - y - (src_h + 1);
 		y_offset = x;
 
-		/*
-		 * TBD: For NV12 90/270 rotation, Y and UV subplanes should
-		 * be treated as separate surfaces and GTT remapping for
-		 * rotation should be done separately for each subplane.
-		 * Enable support once seperate remappings are available.
-		 */
+		if (fb->pixel_format == DRM_FORMAT_NV12) {
+			u32 uv_tile_height = intel_tile_height(dev, fb->pixel_format,
+						fb->modifier[0], 1);
+			aux_stride = DIV_ROUND_UP(fb->height / 2, uv_tile_height);
+			aux_dist = intel_plane_obj_offset(intel_plane, obj, 1) - surf_addr;
+			aux_x_offset = aux_stride * uv_tile_height - y / 2 - fb->height / 2;
+			aux_y_offset = x / 2;
+		}
 	} else {
 		stride = fb->pitches[0] / stride_div;
 		plane_size = (src_h << 16) | src_w;
@@ -904,18 +907,33 @@ intel_check_sprite_plane(struct drm_plane *plane,
 		src_h = drm_rect_height(src) >> 16;
 
 		if (intel_format_is_yuv(fb->pixel_format)) {
-			src_x &= ~1;
-			src_w &= ~1;
-
-			/*
-			 * Must keep src and dst the
-			 * same if we can't scale.
-			 */
-			if (!can_scale)
-				crtc_w &= ~1;
-
-			if (crtc_w == 0)
-				state->visible = false;
+			if (intel_rotation_90_or_270(state->base.rotation)) {
+				src_y &= ~1;
+				src_h &= ~1;
+
+				/*
+				 * Must keep src and dst the
+				 * same if we can't scale.
+				 */
+				if (!can_scale)
+					crtc_h &= ~1;
+
+				if (crtc_h == 0)
+					state->visible = false;
+			} else {
+				src_x &= ~1;
+				src_w &= ~1;
+
+				/*
+				 * Must keep src and dst the
+				 * same if we can't scale.
+				 */
+				if (!can_scale)
+					crtc_w &= ~1;
+
+				if (crtc_w == 0)
+					state->visible = false;
+			}
 		}
 	}
 
-- 
1.7.9.5

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

[PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Chandra Konduru]

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

v2:
-Fix an issue in checks added (me)

v3:
-cosmetic update, split checks into two (Ville)

v4:
-Add stride alignment and modifier checks for UV subplane (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_display.c |   67 ++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |    2 +-
 drivers/gpu/drm/i915/intel_sprite.c  |    2 +-
 3 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 84dad95..6124339 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2906,9 +2906,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 }
 
 u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
-			      uint32_t pixel_format)
+			      uint32_t pixel_format, int plane)
 {
-	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
+	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, plane) * 8;
 
 	/*
 	 * The stride is either expressed as a multiple of 64 bytes
@@ -3117,7 +3117,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 
 	obj = intel_fb_obj(fb);
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
-					       fb->pixel_format);
+					       fb->pixel_format, 0);
 	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
 
 	/*
@@ -9101,7 +9101,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 
 	val = I915_READ(PLANE_STRIDE(pipe, 0));
 	stride_mult = intel_fb_stride_alignment(dev, fb->modifier[0],
-						fb->pixel_format);
+						fb->pixel_format, 0);
 	fb->pitches[0] = (val & 0x3ff) * stride_mult;
 
 	aligned_height = intel_fb_align_height(dev, fb->height,
@@ -11172,7 +11172,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
 	 */
 	stride = fb->pitches[0] /
 		 intel_fb_stride_alignment(dev, fb->modifier[0],
-					   fb->pixel_format);
+					   fb->pixel_format, 0);
 
 	/*
 	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
@@ -14238,6 +14238,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
 {
 	unsigned int aligned_height;
 	int ret;
+	int i;
 	u32 pitch_limit, stride_alignment;
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -14277,12 +14278,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
-						     mode_cmd->pixel_format);
-	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
-		DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
-			  mode_cmd->pitches[0], stride_alignment);
-		return -EINVAL;
+	/* check stride alignment for sub-planes */
+	for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
+		stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[i],
+						     mode_cmd->pixel_format, i);
+		if (mode_cmd->pitches[i] & (stride_alignment - 1)) {
+			DRM_DEBUG("subplane %d pitch (%d) must be at least %u bytes "
+				"aligned\n", i, mode_cmd->pitches[i], stride_alignment);
+			return -EINVAL;
+		}
 	}
 
 	pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
@@ -14349,9 +14353,48 @@ static int intel_framebuffer_init(struct drm_device *dev,
 			return -EINVAL;
 		}
 		break;
+	case DRM_FORMAT_NV12:
+		if (INTEL_INFO(dev)->gen < 9) {
+			DRM_DEBUG("unsupported pixel format: %s\n",
+				drm_get_format_name(mode_cmd->pixel_format));
+			return -EINVAL;
+		}
+		if (obj->tiling_mode == I915_TILING_X &&
+			!(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {
+			mode_cmd->modifier[1] = I915_FORMAT_MOD_X_TILED;
+		}
+		if (!mode_cmd->offsets[1]) {
+			DRM_DEBUG("uv start offset not set\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
+			DRM_DEBUG("y and uv subplanes have different pitches\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->handles[0] != mode_cmd->handles[1]) {
+			DRM_DEBUG("y and uv subplanes have different handles\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[0] != mode_cmd->modifier[1]) {
+			DRM_DEBUG("y and uv subplanes have different modifiers\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
+			(mode_cmd->offsets[1] & 0xFFF)) {
+			DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new tile-row\n",
+				mode_cmd->offsets[1]);
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
+			((mode_cmd->offsets[1] / mode_cmd->pitches[1]) % 4)) {
+			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line aligned\n",
+				mode_cmd->offsets[1]);
+			return -EINVAL;
+		}
+		break;
 	default:
 		DRM_DEBUG("unsupported pixel format: %s\n",
-			  drm_get_format_name(mode_cmd->pixel_format));
+			drm_get_format_name(mode_cmd->pixel_format));
 		return -EINVAL;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d50b8cb..62d2a11 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -980,7 +980,7 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
 void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
 			enum fb_op_origin origin);
 u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
-			      uint32_t pixel_format);
+			      uint32_t pixel_format, int plane);
 
 /* intel_audio.c */
 void intel_init_audio(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 797594e..49feae0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -203,7 +203,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 				       src_w != crtc_w || src_h != crtc_h);
 
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
-					       fb->pixel_format);
+					       fb->pixel_format, 0);
 
 	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
 
-- 
1.7.9.5

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

[PATCH 14/15] drm/i915: skl nv12 wa - NV12 to RGB switch

[Chandra Konduru]

Switching format from NV12 to RGB can result in display underrun
and corruption. This workaround sets bits 15 & 19 to 1 in
CLKGATE_DIS_PSL register to address transition underrun.

v2:
-Move workaround to init clock gating (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |    8 ++++++++
 drivers/gpu/drm/i915/intel_pm.c |    8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d20f235..2e2636d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5366,6 +5366,14 @@ enum skl_disp_power_wells {
 #define CHICKEN_DCPR_1             0x46430
 #define IDLE_WAKEMEM_MASK          (1 << 13)
 
+#define CLKGATE_DIS_PSL_A        0x46520
+#define CLKGATE_DIS_PSL_B        0x46524
+#define CLKGATE_DIS_PSL_C        0x46528
+#define DUPS1_GATING_DIS         (1 << 15)
+#define DUPS2_GATING_DIS         (1 << 19)
+#define DUPS3_GATING_DIS         (1 << 23)
+#define CLKGATE_DIS_PSL(pipe)  _PIPE(pipe, CLKGATE_DIS_PSL_A, CLKGATE_DIS_PSL_B)
+
 /* SKL new cursor registers */
 #define _CUR_BUF_CFG_A				0x7017c
 #define _CUR_BUF_CFG_B				0x7117c
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8a36ab9..9a3deed 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -63,6 +63,14 @@ static void gen9_init_clock_gating(struct drm_device *dev)
 	/* WaDisableKillLogic:bxt,skl */
 	I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) |
 		   ECOCHK_DIS_TLB);
+
+	/*
+	 * Switching format from NV12 to RGB can result in display underrun
+	 * and corruption. This workaround sets bits 15 & 19 to 1 in
+	 * CLKGATE_DIS_PSL register to address transition underrun.
+	 */
+	I915_WRITE(CLKGATE_DIS_PSL(PIPE_A), DUPS1_GATING_DIS | DUPS2_GATING_DIS);
+	I915_WRITE(CLKGATE_DIS_PSL(PIPE_B), DUPS1_GATING_DIS | DUPS2_GATING_DIS);
 }
 
 static void skl_init_clock_gating(struct drm_device *dev)
-- 
1.7.9.5

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

Re: [PATCH 04/15] drm/i915: Stage scaler request for NV12 as src format

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:33:00PM -0700, Chandra Konduru wrote:
> This patch stages a scaler request when input format
> is NV12. The same scaler does both chroma-upsampling
> and resolution scaling as needed.
> 
> v2:
> -Added helper function for need_scaling (Ville)
> 
> v3:
> -Rebased to current kernel version 4.2.0.rc4 (me)
> 
> v4:
> -minor updates (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3ee1c17..8869779 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4341,20 +4341,38 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
>  	}
>  }
>  
> +static bool skl_need_scaling(int src_w, int src_h, int dst_w, int dst_h,
> +	unsigned int rotation, uint32_t pixel_format)
> +{
> +	/* need a scaler when sizes doesn't match */
> +	if (src_w != dst_w || src_h != dst_h)
> +		return true;
> +
> +	/* in case of 90/270 rotation, check src width with dst height and so */
> +	if (intel_rotation_90_or_270(rotation) &&
> +			(src_h != dst_w || src_w != dst_h))
> +		return true;
> +
> +	/* need a scaler for nv12 */
> +	if (pixel_format == DRM_FORMAT_NV12)
> +		return true;
> +
> +	return false;
> +}

That still looks wrong to me. Let me repeat how I think it should look:

{
	if (format == NV12)
		return true;

	if (90_or_270())
		return src_w != dst_h || src_h != dst_w;
	else
		return src_w != dst_w || src_h != dst_h;
}

Otherwise looks good.

> +
>  static int
>  skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>  		  unsigned scaler_user, int *scaler_id, unsigned int rotation,
> -		  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;
>  	struct intel_crtc *intel_crtc =
>  		to_intel_crtc(crtc_state->base.crtc);
> -	int need_scaling;
> +	bool need_scaling;
>  
> -	need_scaling = intel_rotation_90_or_270(rotation) ?
> -		(src_h != dst_w || src_w != dst_h):
> -		(src_w != dst_w || src_h != dst_h);
> +	need_scaling = skl_need_scaling(src_w, src_h, dst_w, dst_h, rotation,
> +		pixel_format);
>  
>  	/*
>  	 * if plane is being disabled or scaler is no more required or force detach
> @@ -4423,7 +4441,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, DRM_ROTATE_0,
>  		state->pipe_src_w, state->pipe_src_h,
> -		adjusted_mode->hdisplay, adjusted_mode->vdisplay);
> +		adjusted_mode->hdisplay, adjusted_mode->vdisplay, 0);
>  }
>  
>  /**
> @@ -4459,7 +4477,8 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
>  				drm_rect_width(&plane_state->src) >> 16,
>  				drm_rect_height(&plane_state->src) >> 16,
>  				drm_rect_width(&plane_state->dst),
> -				drm_rect_height(&plane_state->dst));
> +				drm_rect_height(&plane_state->dst),
> +				fb ? fb->pixel_format : 0);
>  
>  	if (ret || plane_state->scaler_id < 0)
>  		return ret;
> @@ -4484,6 +4503,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] FB:%d unsupported scaling format 0x%x\n",
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:33:03PM -0700, Chandra Konduru wrote:
> This patch adds NV12 to list of supported formats for
> primary plane.
> 
> v2:
> -Rebased (me)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12
> ---
>  drivers/gpu/drm/i915/intel_display.c |   22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3f9111e..84dad95 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -74,6 +74,19 @@ static const uint32_t skl_primary_formats[] = {
>  	DRM_FORMAT_XBGR2101010,
>  };
>  
> +/* Primary plane formats for gen >= 9 with NV12 */
> +static const uint32_t skl_primary_formats_with_nv12[] = {
> +	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_NV12,
> +};

Looks like we've since gained support for packed YUV formats, so this
patch needs a respin to include those as well.

> +
>  /* Cursor formats */
>  static const uint32_t intel_cursor_formats[] = {
>  	DRM_FORMAT_ARGB8888,
> @@ -13612,8 +13625,13 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  		primary->plane = !pipe;
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
> -		intel_primary_formats = skl_primary_formats;
> -		num_formats = ARRAY_SIZE(skl_primary_formats);
> +		if (pipe == PIPE_A || pipe == PIPE_B) {
> +			intel_primary_formats = skl_primary_formats_with_nv12;
> +			num_formats = ARRAY_SIZE(skl_primary_formats_with_nv12);
> +		} else {
> +			intel_primary_formats = skl_primary_formats;
> +			num_formats = ARRAY_SIZE(skl_primary_formats);
> +		}
>  	} else if (INTEL_INFO(dev)->gen >= 4) {
>  		intel_primary_formats = i965_primary_formats;
>  		num_formats = ARRAY_SIZE(i965_primary_formats);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Ville Syrjälä]

On Wed, Sep 09, 2015 at 03:59:03PM -0700, Chandra Konduru wrote:
> This patch adds NV12 as supported format to
> intel_framebuffer_init and performs various checks.
> 
> v2:
> -Fix an issue in checks added (me)
> 
> v3:
> -cosmetic update, split checks into two (Ville)
> 
> v4:
> -Add stride alignment and modifier checks for UV subplane (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12
> ---
>  drivers/gpu/drm/i915/intel_display.c |   67 ++++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |    2 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |    2 +-
>  3 files changed, 57 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 84dad95..6124339 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2906,9 +2906,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  }
>  
>  u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> -			      uint32_t pixel_format)
> +			      uint32_t pixel_format, int plane)
>  {
> -	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
> +	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, plane) * 8;
>  
>  	/*
>  	 * The stride is either expressed as a multiple of 64 bytes
> @@ -3117,7 +3117,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  
>  	obj = intel_fb_obj(fb);
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> -					       fb->pixel_format);
> +					       fb->pixel_format, 0);
>  	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
>  
>  	/*
> @@ -9101,7 +9101,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  
>  	val = I915_READ(PLANE_STRIDE(pipe, 0));
>  	stride_mult = intel_fb_stride_alignment(dev, fb->modifier[0],
> -						fb->pixel_format);
> +						fb->pixel_format, 0);
>  	fb->pitches[0] = (val & 0x3ff) * stride_mult;
>  
>  	aligned_height = intel_fb_align_height(dev, fb->height,
> @@ -11172,7 +11172,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	 */
>  	stride = fb->pitches[0] /
>  		 intel_fb_stride_alignment(dev, fb->modifier[0],
> -					   fb->pixel_format);
> +					   fb->pixel_format, 0);
>  
>  	/*
>  	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> @@ -14238,6 +14238,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  {
>  	unsigned int aligned_height;
>  	int ret;
> +	int i;
>  	u32 pitch_limit, stride_alignment;
>  
>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> @@ -14277,12 +14278,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  		return -EINVAL;
>  	}
>  
> -	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
> -						     mode_cmd->pixel_format);
> -	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
> -		DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
> -			  mode_cmd->pitches[0], stride_alignment);
> -		return -EINVAL;
> +	/* check stride alignment for sub-planes */
> +	for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
> +		stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[i],
> +						     mode_cmd->pixel_format, i);
> +		if (mode_cmd->pitches[i] & (stride_alignment - 1)) {
> +			DRM_DEBUG("subplane %d pitch (%d) must be at least %u bytes "
> +				"aligned\n", i, mode_cmd->pitches[i], stride_alignment);
> +			return -EINVAL;
> +		}
>  	}
>  
>  	pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
> @@ -14349,9 +14353,48 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  			return -EINVAL;
>  		}
>  		break;
> +	case DRM_FORMAT_NV12:
> +		if (INTEL_INFO(dev)->gen < 9) {
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +				drm_get_format_name(mode_cmd->pixel_format));
> +			return -EINVAL;
> +		}
> +		if (obj->tiling_mode == I915_TILING_X &&
> +			!(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {

Your editor still seems to mess up the indentation in these cases. Can
you try to fix that?

> +			mode_cmd->modifier[1] = I915_FORMAT_MOD_X_TILED;
> +		}

Hmm. This obj tiling -> modifier conversion should be a fairly generic
thing to do, so I suggest doing it for all planes in one place. Maybe
add a new function intel_fb_obj_tiling_to_modifier() or something like
that and loop over all the planes there.

> +		if (!mode_cmd->offsets[1]) {
> +			DRM_DEBUG("uv start offset not set\n");
> +			return -EINVAL;
> +		}

Still not really happy with this check since it's either too limiting, or
not restrictive enough. Depending on how you look at it. Do you know
if the hardware gets upset if you tell it use overlapping Y and UV
surfaces?

> +		if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> +			DRM_DEBUG("y and uv subplanes have different pitches\n");
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->handles[0] != mode_cmd->handles[1]) {
> +			DRM_DEBUG("y and uv subplanes have different handles\n");
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->modifier[0] != mode_cmd->modifier[1]) {
> +			DRM_DEBUG("y and uv subplanes have different modifiers\n");
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
> +			(mode_cmd->offsets[1] & 0xFFF)) {
> +			DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new tile-row\n",
> +				mode_cmd->offsets[1]);
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
> +			((mode_cmd->offsets[1] / mode_cmd->pitches[1]) % 4)) {
> +			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line aligned\n",
> +				mode_cmd->offsets[1]);
> +			return -EINVAL;
> +		}

I was going to say I can't find anything in the spec to support this,
but after some more reading I got it "The display hardware requires
that the UV surface start satisfies four line alignment from the
begining of the page." So the check should be something like
((offsets[1] & 0xfff) / pitches[1] % 4.

However we should anyway be able to adjust the X/Y offsets to account
for misalignment of offsets[1]. I think the only thing we should need
to check is that offsets[1] is macropixel aligned. The patch lacks such
a check in fact.

Feels like it would be time to expand intel_gen4_compute_page_offset()
to handle the new tiling formats and start using it for SKL+. Though
rotation may need some additional thought. Also maybe it's time to
dig up my old offsets[0] handling patch and refresh it a bit and try
to get it merged again.

> +		break;
>  	default:
>  		DRM_DEBUG("unsupported pixel format: %s\n",
> -			  drm_get_format_name(mode_cmd->pixel_format));
> +			drm_get_format_name(mode_cmd->pixel_format));
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d50b8cb..62d2a11 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -980,7 +980,7 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
>  void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
>  			enum fb_op_origin origin);
>  u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> -			      uint32_t pixel_format);
> +			      uint32_t pixel_format, int plane);
>  
>  /* intel_audio.c */
>  void intel_init_audio(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 797594e..49feae0 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -203,7 +203,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  				       src_w != crtc_w || src_h != crtc_h);
>  
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> -					       fb->pixel_format);
> +					       fb->pixel_format, 0);
>  
>  	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/15] drm/i915: Stage scaler request for NV12 as src format

[Konduru, Chandra]

> > +static bool skl_need_scaling(int src_w, int src_h, int dst_w, int dst_h,
> > +	unsigned int rotation, uint32_t pixel_format)
> > +{
> > +	/* need a scaler when sizes doesn't match */
> > +	if (src_w != dst_w || src_h != dst_h)
> > +		return true;
> > +
> > +	/* in case of 90/270 rotation, check src width with dst height and so */
> > +	if (intel_rotation_90_or_270(rotation) &&
> > +			(src_h != dst_w || src_w != dst_h))
> > +		return true;
> > +
> > +	/* need a scaler for nv12 */
> > +	if (pixel_format == DRM_FORMAT_NV12)
> > +		return true;
> > +
> > +	return false;
> > +}
> 
> That still looks wrong to me. Let me repeat how I think it should look:
> 
> {
> 	if (format == NV12)
> 		return true;
> 
> 	if (90_or_270())
> 		return src_w != dst_h || src_h != dst_w;
> 	else
> 		return src_w != dst_w || src_h != dst_h;
> }
> 
> Otherwise looks good.
Sending updated patch shortly.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Konduru, Chandra]

> > +		if (obj->tiling_mode == I915_TILING_X &&
> > +			!(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {
> 
> Your editor still seems to mess up the indentation in these cases. Can
> you try to fix that?

If condition isn't fitting in a single line, so continued in next line like any other if condition.
Not sure what the exact indentation issue you are observing.

> 
> > +			mode_cmd->modifier[1] = I915_FORMAT_MOD_X_TILED;
> > +		}
> 
> Hmm. This obj tiling -> modifier conversion should be a fairly generic
> thing to do, so I suggest doing it for all planes in one place. Maybe
> add a new function intel_fb_obj_tiling_to_modifier() or something like
> that and loop over all the planes there.

Then will merge this assignment into beginning of the function
which does for modifier[0]. Will make a loop there. 
Is that ok?

> 
> > +		if (!mode_cmd->offsets[1]) {
> > +			DRM_DEBUG("uv start offset not set\n");
> > +			return -EINVAL;
> > +		}
> 
> Still not really happy with this check since it's either too limiting, or
> not restrictive enough. Depending on how you look at it. Do you know
> if the hardware gets upset if you tell it use overlapping Y and UV
> surfaces?

May be not.
But setting NV12 UV offset from userland can be a miss. So added
this check to help userland knowing the issue.

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

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Ville Syrjälä]

On Thu, Sep 10, 2015 at 07:14:58PM +0000, Konduru, Chandra wrote:
> > > +		if (obj->tiling_mode == I915_TILING_X &&
> > > +			!(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {
> > 
> > Your editor still seems to mess up the indentation in these cases. Can
> > you try to fix that?
> 
> If condition isn't fitting in a single line, so continued in next line like any other if condition.
> Not sure what the exact indentation issue you are observing.

Stuff should line up nicely after the opening (

if (foo &&
    bar &&
    zonk)

and not

if (foo &&
	bar &&
	zonk)


> 
> > 
> > > +			mode_cmd->modifier[1] = I915_FORMAT_MOD_X_TILED;
> > > +		}
> > 
> > Hmm. This obj tiling -> modifier conversion should be a fairly generic
> > thing to do, so I suggest doing it for all planes in one place. Maybe
> > add a new function intel_fb_obj_tiling_to_modifier() or something like
> > that and loop over all the planes there.
> 
> Then will merge this assignment into beginning of the function
> which does for modifier[0]. Will make a loop there. 
> Is that ok?

Sure.

> 
> > 
> > > +		if (!mode_cmd->offsets[1]) {
> > > +			DRM_DEBUG("uv start offset not set\n");
> > > +			return -EINVAL;
> > > +		}
> > 
> > Still not really happy with this check since it's either too limiting, or
> > not restrictive enough. Depending on how you look at it. Do you know
> > if the hardware gets upset if you tell it use overlapping Y and UV
> > surfaces?
> 
> May be not.
> But setting NV12 UV offset from userland can be a miss. So added
> this check to help userland knowing the issue.

It only catches the 0 case though. I think if userspace would forget to
set offsets[1] it would most likely also forget handles[1], so I think
we should be fairly well covered against the most obvious userspace
fumbles.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Ville Syrjälä]

On Thu, Sep 10, 2015 at 09:34:59PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 09, 2015 at 03:59:03PM -0700, Chandra Konduru wrote:
> > This patch adds NV12 as supported format to
> > intel_framebuffer_init and performs various checks.
> > 
> > v2:
> > -Fix an issue in checks added (me)
> > 
> > v3:
> > -cosmetic update, split checks into two (Ville)
> > 
> > v4:
> > -Add stride alignment and modifier checks for UV subplane (Ville)
> > 
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Testcase: igt/kms_nv12
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |   67 ++++++++++++++++++++++++++++------
> >  drivers/gpu/drm/i915/intel_drv.h     |    2 +-
> >  drivers/gpu/drm/i915/intel_sprite.c  |    2 +-
> >  3 files changed, 57 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 84dad95..6124339 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2906,9 +2906,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> >  }
> >  
> >  u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> > -			      uint32_t pixel_format)
> > +			      uint32_t pixel_format, int plane)
> >  {
> > -	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
> > +	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, plane) * 8;
> >  
> >  	/*
> >  	 * The stride is either expressed as a multiple of 64 bytes
> > @@ -3117,7 +3117,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> >  
> >  	obj = intel_fb_obj(fb);
> >  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> > -					       fb->pixel_format);
> > +					       fb->pixel_format, 0);
> >  	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
> >  
> >  	/*
> > @@ -9101,7 +9101,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
> >  
> >  	val = I915_READ(PLANE_STRIDE(pipe, 0));
> >  	stride_mult = intel_fb_stride_alignment(dev, fb->modifier[0],
> > -						fb->pixel_format);
> > +						fb->pixel_format, 0);
> >  	fb->pitches[0] = (val & 0x3ff) * stride_mult;
> >  
> >  	aligned_height = intel_fb_align_height(dev, fb->height,
> > @@ -11172,7 +11172,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
> >  	 */
> >  	stride = fb->pitches[0] /
> >  		 intel_fb_stride_alignment(dev, fb->modifier[0],
> > -					   fb->pixel_format);
> > +					   fb->pixel_format, 0);
> >  
> >  	/*
> >  	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> > @@ -14238,6 +14238,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >  {
> >  	unsigned int aligned_height;
> >  	int ret;
> > +	int i;
> >  	u32 pitch_limit, stride_alignment;
> >  
> >  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> > @@ -14277,12 +14278,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >  		return -EINVAL;
> >  	}
> >  
> > -	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
> > -						     mode_cmd->pixel_format);
> > -	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
> > -		DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
> > -			  mode_cmd->pitches[0], stride_alignment);
> > -		return -EINVAL;
> > +	/* check stride alignment for sub-planes */
> > +	for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
> > +		stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[i],
> > +						     mode_cmd->pixel_format, i);
> > +		if (mode_cmd->pitches[i] & (stride_alignment - 1)) {
> > +			DRM_DEBUG("subplane %d pitch (%d) must be at least %u bytes "
> > +				"aligned\n", i, mode_cmd->pitches[i], stride_alignment);
> > +			return -EINVAL;
> > +		}
> >  	}
> >  
> >  	pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
> > @@ -14349,9 +14353,48 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >  			return -EINVAL;
> >  		}
> >  		break;
> > +	case DRM_FORMAT_NV12:
> > +		if (INTEL_INFO(dev)->gen < 9) {
> > +			DRM_DEBUG("unsupported pixel format: %s\n",
> > +				drm_get_format_name(mode_cmd->pixel_format));
> > +			return -EINVAL;
> > +		}
> > +		if (obj->tiling_mode == I915_TILING_X &&
> > +			!(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {
> 
> Your editor still seems to mess up the indentation in these cases. Can
> you try to fix that?
> 
> > +			mode_cmd->modifier[1] = I915_FORMAT_MOD_X_TILED;
> > +		}
> 
> Hmm. This obj tiling -> modifier conversion should be a fairly generic
> thing to do, so I suggest doing it for all planes in one place. Maybe
> add a new function intel_fb_obj_tiling_to_modifier() or something like
> that and loop over all the planes there.
> 
> > +		if (!mode_cmd->offsets[1]) {
> > +			DRM_DEBUG("uv start offset not set\n");
> > +			return -EINVAL;
> > +		}
> 
> Still not really happy with this check since it's either too limiting, or
> not restrictive enough. Depending on how you look at it. Do you know
> if the hardware gets upset if you tell it use overlapping Y and UV
> surfaces?
> 
> > +		if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> > +			DRM_DEBUG("y and uv subplanes have different pitches\n");
> > +			return -EINVAL;
> > +		}
> > +		if (mode_cmd->handles[0] != mode_cmd->handles[1]) {
> > +			DRM_DEBUG("y and uv subplanes have different handles\n");
> > +			return -EINVAL;
> > +		}
> > +		if (mode_cmd->modifier[0] != mode_cmd->modifier[1]) {
> > +			DRM_DEBUG("y and uv subplanes have different modifiers\n");
> > +			return -EINVAL;
> > +		}
> > +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
> > +			(mode_cmd->offsets[1] & 0xFFF)) {
> > +			DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new tile-row\n",
> > +				mode_cmd->offsets[1]);
> > +			return -EINVAL;
> > +		}
> > +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
> > +			((mode_cmd->offsets[1] / mode_cmd->pitches[1]) % 4)) {
> > +			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line aligned\n",
> > +				mode_cmd->offsets[1]);
> > +			return -EINVAL;
> > +		}
> 
> I was going to say I can't find anything in the spec to support this,
> but after some more reading I got it "The display hardware requires
> that the UV surface start satisfies four line alignment from the
> begining of the page." So the check should be something like
> ((offsets[1] & 0xfff) / pitches[1] % 4.
> 
> However we should anyway be able to adjust the X/Y offsets to account
> for misalignment of offsets[1]. I think the only thing we should need
> to check is that offsets[1] is macropixel aligned. The patch lacks such
> a check in fact.
> 
> Feels like it would be time to expand intel_gen4_compute_page_offset()
> to handle the new tiling formats and start using it for SKL+. Though
> rotation may need some additional thought. Also maybe it's time to
> dig up my old offsets[0] handling patch and refresh it a bit and try
> to get it merged again.

Just to clarify a bit. I think we can initially go with the checks in
place, and we can work on refactoring the page offset stuff afterwards
Actually I already started to sketch something together here :) so I
might have a few patches to post sooner rather than later.

> 
> > +		break;
> >  	default:
> >  		DRM_DEBUG("unsupported pixel format: %s\n",
> > -			  drm_get_format_name(mode_cmd->pixel_format));
> > +			drm_get_format_name(mode_cmd->pixel_format));
> >  		return -EINVAL;
> >  	}
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index d50b8cb..62d2a11 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -980,7 +980,7 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
> >  void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
> >  			enum fb_op_origin origin);
> >  u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> > -			      uint32_t pixel_format);
> > +			      uint32_t pixel_format, int plane);
> >  
> >  /* intel_audio.c */
> >  void intel_init_audio(struct drm_device *dev);
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 797594e..49feae0 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -203,7 +203,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> >  				       src_w != crtc_w || src_h != crtc_h);
> >  
> >  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> > -					       fb->pixel_format);
> > +					       fb->pixel_format, 0);
> >  
> >  	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
> >  
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Konduru, Chandra]

> > > > +		if (obj->tiling_mode == I915_TILING_X &&
> > > > +			!(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {
> > >
> > > Your editor still seems to mess up the indentation in these cases. Can
> > > you try to fix that?
> >
> > If condition isn't fitting in a single line, so continued in next line like any other if
> condition.
> > Not sure what the exact indentation issue you are observing.
> 
> Stuff should line up nicely after the opening (
> 
> if (foo &&
>     bar &&
>     zonk)
> 
> and not
> 
> if (foo &&
> 	bar &&
> 	zonk)

I am continuing tab to indent the line below "if" to move past "if".
In the code, I see "space" or "tab" to move past "if" in line below.
One or the other, I used tab. But if that is causing issue, it is
Fine to use "spaces".

Anyway, this code is going away, as it is being merged into
beginning of the function.

> 
> 
> >
> > >
> > > > +			mode_cmd->modifier[1] =
> I915_FORMAT_MOD_X_TILED;
> > > > +		}
> > >
> > > Hmm. This obj tiling -> modifier conversion should be a fairly generic
> > > thing to do, so I suggest doing it for all planes in one place. Maybe
> > > add a new function intel_fb_obj_tiling_to_modifier() or something like
> > > that and loop over all the planes there.
> >
> > Then will merge this assignment into beginning of the function
> > which does for modifier[0]. Will make a loop there.
> > Is that ok?
> 
> Sure.
> 
> >
> > >
> > > > +		if (!mode_cmd->offsets[1]) {
> > > > +			DRM_DEBUG("uv start offset not set\n");
> > > > +			return -EINVAL;
> > > > +		}
> > >
> > > Still not really happy with this check since it's either too limiting, or
> > > not restrictive enough. Depending on how you look at it. Do you know
> > > if the hardware gets upset if you tell it use overlapping Y and UV
> > > surfaces?
> >
> > May be not.
> > But setting NV12 UV offset from userland can be a miss. So added
> > this check to help userland knowing the issue.
> 
> It only catches the 0 case though. I think if userspace would forget to
> set offsets[1] it would most likely also forget handles[1], so I think
> we should be fairly well covered against the most obvious userspace
> fumbles.
Yes, both handles and offsets are covered. So keeping this if block.

> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Konduru, Chandra]

> > > +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED
> &&
> > > +			((mode_cmd->offsets[1] / mode_cmd->pitches[1]) %
> 4)) {
> > > +			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line
> aligned\n",
> > > +				mode_cmd->offsets[1]);
> > > +			return -EINVAL;
> > > +		}
> >
> > I was going to say I can't find anything in the spec to support this,
> > but after some more reading I got it "The display hardware requires
> > that the UV surface start satisfies four line alignment from the
> > begining of the page." So the check should be something like
> > ((offsets[1] & 0xfff) / pitches[1] % 4.

Yes, that is the bspec documentation for this check.
It is requiring the uv-offset to be 4-line aligned i.e., checking
uv-offset that is given to driver. If you take away 12-lsbs
then it is already at the page start. Why you want to take
away 12-LSBs?

> >
> > However we should anyway be able to adjust the X/Y offsets to account
> > for misalignment of offsets[1]. I think the only thing we should need
> > to check is that offsets[1] is macropixel aligned. The patch lacks such
> > a check in fact.

UV-start is with respect to buffer and not relate to any incoming flip
clipping the uv where macro-pixel adjustments are done.
Sorry, I didn't get why you think X/Y offset maco-pixel adjustments 
have anything to do with uv-offset checking here in this if block?
 
> >
> > Feels like it would be time to expand intel_gen4_compute_page_offset()
> > to handle the new tiling formats and start using it for SKL+. Though
> > rotation may need some additional thought. Also maybe it's time to
> > dig up my old offsets[0] handling patch and refresh it a bit and try
> > to get it merged again.
> 
> Just to clarify a bit. I think we can initially go with the checks in
> place, and we can work on refactoring the page offset stuff afterwards
> Actually I already started to sketch something together here :) so I
> might have a few patches to post sooner rather than later.


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

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

[Konduru, Chandra]

> > +/* Primary plane formats for gen >= 9 with NV12 */
> > +static const uint32_t skl_primary_formats_with_nv12[] = {
> > +	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_NV12,
> > +};
> 
> Looks like we've since gained support for packed YUV formats, so this
> patch needs a respin to include those as well.
> 

A while ago Daniel mentioned he can resolve minor/small 
merge conflicts and don't have to respun. 
So my understanding is this should be ok as it should be 
minor one.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Thu, Sep 10, 2015 at 09:06:23PM +0000, Konduru, Chandra wrote:
> > > +/* Primary plane formats for gen >= 9 with NV12 */
> > > +static const uint32_t skl_primary_formats_with_nv12[] = {
> > > +	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_NV12,
> > > +};
> > 
> > Looks like we've since gained support for packed YUV formats, so this
> > patch needs a respin to include those as well.
> > 
> 
> A while ago Daniel mentioned he can resolve minor/small 
> merge conflicts and don't have to respun. 
> So my understanding is this should be ok as it should be 
> minor one.

Something like this may not even show up as a conflict since you're just
adding a new table. And even if it does, it would be very easy to
overlook the fact that the new table should also have the 4:2:2 formats.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Konduru, Chandra]

> > > > +/* Primary plane formats for gen >= 9 with NV12 */
> > > > +static const uint32_t skl_primary_formats_with_nv12[] = {
> > > > +	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_NV12,
> > > > +};
> > >
> > > Looks like we've since gained support for packed YUV formats, so this
> > > patch needs a respin to include those as well.
> > >
> >
> > A while ago Daniel mentioned he can resolve minor/small
> > merge conflicts and don't have to respun.
> > So my understanding is this should be ok as it should be
> > minor one.
> 
> Something like this may not even show up as a conflict since you're just
> adding a new table. And even if it does, it would be very easy to
> overlook the fact that the new table should also have the 4:2:2 formats.

Ok, will respun this patch with YUV packed formats patch being applied.


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

[PATCH 04/15] drm/i915: Stage scaler request for NV12 as src format

[Chandra Konduru]

This patch stages a scaler request when input format
is NV12. The same scaler does both chroma-upsampling
and resolution scaling as needed.

v2:
-Added helper function for need_scaling (Ville)

v3:
-Rebased to current kernel version 4.2.0.rc4 (me)

v4:
-minor updates (Ville)

v5:
-updated scaler helper function (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 356f071..cf6c31d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4345,20 +4345,33 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 	}
 }
 
+static bool skl_need_scaling(int src_w, int src_h, int dst_w, int dst_h,
+	unsigned int rotation, uint32_t pixel_format)
+{
+	/* need a scaler for nv12 */
+	if (pixel_format == DRM_FORMAT_NV12)
+		return true;
+
+	/* need a scaler when sizes doesn't match */
+	if (intel_rotation_90_or_270(rotation))
+		return (src_h != dst_w || src_w != dst_h);
+	else
+		return (src_w != dst_w || src_h != dst_h);
+}
+
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		  unsigned scaler_user, int *scaler_id, unsigned int rotation,
-		  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;
 	struct intel_crtc *intel_crtc =
 		to_intel_crtc(crtc_state->base.crtc);
-	int need_scaling;
+	bool need_scaling;
 
-	need_scaling = intel_rotation_90_or_270(rotation) ?
-		(src_h != dst_w || src_w != dst_h):
-		(src_w != dst_w || src_h != dst_h);
+	need_scaling = skl_need_scaling(src_w, src_h, dst_w, dst_h, rotation,
+		pixel_format);
 
 	/*
 	 * if plane is being disabled or scaler is no more required or force detach
@@ -4427,7 +4440,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, DRM_ROTATE_0,
 		state->pipe_src_w, state->pipe_src_h,
-		adjusted_mode->hdisplay, adjusted_mode->vdisplay);
+		adjusted_mode->hdisplay, adjusted_mode->vdisplay, 0);
 }
 
 /**
@@ -4463,7 +4476,8 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 				drm_rect_width(&plane_state->src) >> 16,
 				drm_rect_height(&plane_state->src) >> 16,
 				drm_rect_width(&plane_state->dst),
-				drm_rect_height(&plane_state->dst));
+				drm_rect_height(&plane_state->dst),
+				fb ? fb->pixel_format : 0);
 
 	if (ret || plane_state->scaler_id < 0)
 		return ret;
@@ -4488,6 +4502,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] FB:%d unsupported scaling format 0x%x\n",
-- 
1.7.9.5

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

[PATCH 07/15] drm/i915: Add NV12 as supported format for primary plane

[Chandra Konduru]

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

v2:
-Rebased (me)

v3:
-Rebased on top of primary plane YUV support patch (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 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 4122359..2a5170e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -78,6 +78,23 @@ static const uint32_t skl_primary_formats[] = {
 	DRM_FORMAT_VYUY,
 };
 
+/* Primary plane formats for gen >= 9 with NV12 */
+static const uint32_t skl_primary_formats_with_nv12[] = {
+	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,
@@ -13611,8 +13628,13 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 		primary->plane = !pipe;
 
 	if (INTEL_INFO(dev)->gen >= 9) {
-		intel_primary_formats = skl_primary_formats;
-		num_formats = ARRAY_SIZE(skl_primary_formats);
+		if (pipe == PIPE_A || pipe == PIPE_B) {
+			intel_primary_formats = skl_primary_formats_with_nv12;
+			num_formats = ARRAY_SIZE(skl_primary_formats_with_nv12);
+		} else {
+			intel_primary_formats = skl_primary_formats;
+			num_formats = ARRAY_SIZE(skl_primary_formats);
+		}
 	} else if (INTEL_INFO(dev)->gen >= 4) {
 		intel_primary_formats = i965_primary_formats;
 		num_formats = ARRAY_SIZE(i965_primary_formats);
-- 
1.7.9.5

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

[PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Chandra Konduru]

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

v2:
-Fix an issue in checks added (me)

v3:
-cosmetic update, split checks into two (Ville)

v4:
-Add stride alignment and modifier checks for UV subplane (Ville)

v5:
-Make modifier check general (Ville)
-Check tile-y uv start alignment from begining of page (Ville)

Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_display.c |   66 +++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h     |    2 +-
 drivers/gpu/drm/i915/intel_sprite.c  |    2 +-
 3 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a5170e..af28ca9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2914,9 +2914,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 }
 
 u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
-			      uint32_t pixel_format)
+			      uint32_t pixel_format, int plane)
 {
-	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
+	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, plane) * 8;
 
 	/*
 	 * The stride is either expressed as a multiple of 64 bytes
@@ -3125,7 +3125,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 
 	obj = intel_fb_obj(fb);
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
-					       fb->pixel_format);
+					       fb->pixel_format, 0);
 	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
 
 	/*
@@ -9104,7 +9104,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
 
 	val = I915_READ(PLANE_STRIDE(pipe, 0));
 	stride_mult = intel_fb_stride_alignment(dev, fb->modifier[0],
-						fb->pixel_format);
+						fb->pixel_format, 0);
 	fb->pitches[0] = (val & 0x3ff) * stride_mult;
 
 	aligned_height = intel_fb_align_height(dev, fb->height,
@@ -11175,7 +11175,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
 	 */
 	stride = fb->pitches[0] /
 		 intel_fb_stride_alignment(dev, fb->modifier[0],
-					   fb->pixel_format);
+					   fb->pixel_format, 0);
 
 	/*
 	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
@@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
 {
 	unsigned int aligned_height;
 	int ret;
+	int i;
 	u32 pitch_limit, stride_alignment;
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
@@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct drm_device *dev,
 		}
 	} else {
 		if (obj->tiling_mode == I915_TILING_X)
-			mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
+			for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++)
+				mode_cmd->modifier[i] = I915_FORMAT_MOD_X_TILED;
 		else if (obj->tiling_mode == I915_TILING_Y) {
 			DRM_DEBUG("No Y tiling for legacy addfb\n");
 			return -EINVAL;
@@ -14280,12 +14282,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
 		return -EINVAL;
 	}
 
-	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
-						     mode_cmd->pixel_format);
-	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
-		DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
-			  mode_cmd->pitches[0], stride_alignment);
-		return -EINVAL;
+	/* check stride alignment for sub-planes */
+	for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
+		stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[i],
+						     mode_cmd->pixel_format, i);
+		if (mode_cmd->pitches[i] & (stride_alignment - 1)) {
+			DRM_DEBUG("subplane %d pitch (%d) must be at least %u bytes "
+				"aligned\n", i, mode_cmd->pitches[i], stride_alignment);
+			return -EINVAL;
+		}
 	}
 
 	pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
@@ -14352,9 +14357,44 @@ static int intel_framebuffer_init(struct drm_device *dev,
 			return -EINVAL;
 		}
 		break;
+	case DRM_FORMAT_NV12:
+		if (INTEL_INFO(dev)->gen < 9) {
+			DRM_DEBUG("unsupported pixel format: %s\n",
+				drm_get_format_name(mode_cmd->pixel_format));
+			return -EINVAL;
+		}
+		if (!mode_cmd->offsets[1]) {
+			DRM_DEBUG("uv start offset not set\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
+			DRM_DEBUG("y and uv subplanes have different pitches\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->handles[0] != mode_cmd->handles[1]) {
+			DRM_DEBUG("y and uv subplanes have different handles\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[0] != mode_cmd->modifier[1]) {
+			DRM_DEBUG("y and uv subplanes have different modifiers\n");
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
+			(mode_cmd->offsets[1] & 0xFFF)) {
+			DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new tile-row\n",
+				mode_cmd->offsets[1]);
+			return -EINVAL;
+		}
+		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
+			(((mode_cmd->offsets[1] & 0xFFF) / mode_cmd->pitches[1]) % 4)) {
+			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line aligned\n",
+				mode_cmd->offsets[1]);
+			return -EINVAL;
+		}
+		break;
 	default:
 		DRM_DEBUG("unsupported pixel format: %s\n",
-			  drm_get_format_name(mode_cmd->pixel_format));
+			drm_get_format_name(mode_cmd->pixel_format));
 		return -EINVAL;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d50b8cb..62d2a11 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -980,7 +980,7 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
 void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
 			enum fb_op_origin origin);
 u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
-			      uint32_t pixel_format);
+			      uint32_t pixel_format, int plane);
 
 /* intel_audio.c */
 void intel_init_audio(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 797594e..49feae0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -203,7 +203,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 				       src_w != crtc_w || src_h != crtc_h);
 
 	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
-					       fb->pixel_format);
+					       fb->pixel_format, 0);
 
 	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
 
-- 
1.7.9.5

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

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

[Daniel Vetter]

On Fri, Sep 11, 2015 at 12:28:28AM +0300, Ville Syrjälä wrote:
> On Thu, Sep 10, 2015 at 09:06:23PM +0000, Konduru, Chandra wrote:
> > > > +/* Primary plane formats for gen >= 9 with NV12 */
> > > > +static const uint32_t skl_primary_formats_with_nv12[] = {
> > > > +	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_NV12,
> > > > +};
> > > 
> > > Looks like we've since gained support for packed YUV formats, so this
> > > patch needs a respin to include those as well.
> > > 
> > 
> > A while ago Daniel mentioned he can resolve minor/small 
> > merge conflicts and don't have to respun. 
> > So my understanding is this should be ok as it should be 
> > minor one.
> 
> Something like this may not even show up as a conflict since you're just
> adding a new table. And even if it does, it would be very easy to
> overlook the fact that the new table should also have the 4:2:2 formats.

Minor conflicts = changes around the code you're changing that doesn't
materially affect your patch.

This here seems like a full-blown feature conflict and means we might even
need to retest (and also make sure test coverage is still adequate).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Daniel Vetter]

On Thu, Sep 10, 2015 at 08:45:48PM +0000, Konduru, Chandra wrote:
> > > > > +		if (obj->tiling_mode == I915_TILING_X &&
> > > > > +			!(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) {
> > > >
> > > > Your editor still seems to mess up the indentation in these cases. Can
> > > > you try to fix that?
> > >
> > > If condition isn't fitting in a single line, so continued in next line like any other if
> > condition.
> > > Not sure what the exact indentation issue you are observing.
> > 
> > Stuff should line up nicely after the opening (
> > 
> > if (foo &&
> >     bar &&
> >     zonk)
> > 
> > and not
> > 
> > if (foo &&
> > 	bar &&
> > 	zonk)
> 
> I am continuing tab to indent the line below "if" to move past "if".
> In the code, I see "space" or "tab" to move past "if" in line below.
> One or the other, I used tab. But if that is causing issue, it is
> Fine to use "spaces".
> 
> Anyway, this code is going away, as it is being merged into
> beginning of the function.

Yeah, continuation lines should be aligned with the opening ( whether
that's an if condition, function parameter list or something else.
checkpatch --strict will spot these for you.

I don't mind if it's just the occasional one, but if you get it
consistently wrong it does make the code a bit harder to follow (simply
because it's unexpected).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Konduru, Chandra]

> > On Thu, Sep 10, 2015 at 09:06:23PM +0000, Konduru, Chandra wrote:
> > > > > +/* Primary plane formats for gen >= 9 with NV12 */
> > > > > +static const uint32_t skl_primary_formats_with_nv12[] = {
> > > > > +	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_NV12,
> > > > > +};
> > > >
> > > > Looks like we've since gained support for packed YUV formats, so this
> > > > patch needs a respin to include those as well.
> > > >
> > >
> > > A while ago Daniel mentioned he can resolve minor/small
> > > merge conflicts and don't have to respun.
> > > So my understanding is this should be ok as it should be
> > > minor one.
> >
> > Something like this may not even show up as a conflict since you're just
> > adding a new table. And even if it does, it would be very easy to
> > overlook the fact that the new table should also have the 4:2:2 formats.
> 
> Minor conflicts = changes around the code you're changing that doesn't
> materially affect your patch.
> 
> This here seems like a full-blown feature conflict and means we might even
> need to retest (and also make sure test coverage is still adequate).

Agree, patch has been rebased and updated patch resubmitted after retesting.

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

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Konduru, Chandra]

> > > > > > +		if (obj->tiling_mode == I915_TILING_X &&
> > > > > > +			!(mode_cmd->flags &
> DRM_MODE_FB_MODIFIERS)) {
> > > > >
> > > > > Your editor still seems to mess up the indentation in these cases. Can
> > > > > you try to fix that?
> > > >
> > > > If condition isn't fitting in a single line, so continued in next line like any
> other if
> > > condition.
> > > > Not sure what the exact indentation issue you are observing.
> > >
> > > Stuff should line up nicely after the opening (
> > >
> > > if (foo &&
> > >     bar &&
> > >     zonk)
> > >
> > > and not
> > >
> > > if (foo &&
> > > 	bar &&
> > > 	zonk)
> >
> > I am continuing tab to indent the line below "if" to move past "if".
> > In the code, I see "space" or "tab" to move past "if" in line below.
> > One or the other, I used tab. But if that is causing issue, it is
> > Fine to use "spaces".
> >
> > Anyway, this code is going away, as it is being merged into
> > beginning of the function.
> 
> Yeah, continuation lines should be aligned with the opening ( whether
> that's an if condition, function parameter list or something else.
> checkpatch --strict will spot these for you.
> 
> I don't mind if it's just the occasional one, but if you get it
> consistently wrong it does make the code a bit harder to follow (simply
> because it's unexpected).

Point noted.


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

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Konduru, Chandra]

> > > > +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED
> > &&
> > > > +			((mode_cmd->offsets[1] / mode_cmd->pitches[1]) %
> > 4)) {
> > > > +			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line
> > aligned\n",
> > > > +				mode_cmd->offsets[1]);
> > > > +			return -EINVAL;
> > > > +		}
> > >
> > > I was going to say I can't find anything in the spec to support this,
> > > but after some more reading I got it "The display hardware requires
> > > that the UV surface start satisfies four line alignment from the
> > > begining of the page." So the check should be something like
> > > ((offsets[1] & 0xfff) / pitches[1] % 4.
> > >
> Ignore my previous response for this. Yes, above check should
> check for 12-lsbs. Will update and respun shortly.
Submitted updated patch. 
With this change, resolved all your feedback.
Is there any more feedback?
If not, can you issue R-B tag for the patches?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:32:57PM -0700, Chandra Konduru wrote:
> Properly allocate min blocks per hw requirements.
> 
> v2:
> - changed helper functional param to bool, some code simplification (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>

IIRC I gave my r-b alrady?

But I'll toss it in again:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fff0c22..4d3aca0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2959,6 +2959,31 @@ skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
>  	return total_data_rate;
>  }
>  
> +static uint16_t
> +skl_dbuf_min_alloc(const struct intel_plane_wm_parameters *p, bool y_plane)
> +{
> +	uint16_t min_alloc;
> +
> +	/* For packed formats, no y-plane, return 0 */
> +	if (y_plane && !p->y_bytes_per_pixel)
> +		return 0;
> +
> +	if (p->tiling == I915_FORMAT_MOD_Y_TILED ||
> +	    p->tiling == I915_FORMAT_MOD_Yf_TILED) {
> +		uint32_t min_scanlines = 8;
> +		uint8_t bytes_per_pixel =
> +			y_plane ? p->y_bytes_per_pixel : p->bytes_per_pixel;
> +
> +		min_scanlines = 32 / bytes_per_pixel;
> +		min_alloc = DIV_ROUND_UP((4 * p->horiz_pixels/(y_plane ? 1 : 2) *
> +			bytes_per_pixel), 512) * min_scanlines/4 + 3;
> +	} else {
> +		min_alloc = 8;
> +	}
> +
> +	return min_alloc;
> +}
> +
>  static void
>  skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>  		      const struct intel_wm_config *config,
> @@ -2999,9 +3024,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>  		if (!p->enabled)
>  			continue;
>  
> -		minimum[plane] = 8;
> +		minimum[plane] = skl_dbuf_min_alloc(p, false);   /* uv-plane/packed */
>  		alloc_size -= minimum[plane];
> -		y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
> +		y_minimum[plane] = skl_dbuf_min_alloc(p, true);  /* y-plane */
>  		alloc_size -= y_minimum[plane];
>  	}
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:32:58PM -0700, Chandra Konduru wrote:
> This patch swaps src width and height for dbuf/wm calculations
> when rotation is 90/270 as per hw requirements.
> 
> v2:
> - minor/cosmetic changes, removed plane_state check kludge (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>

Spec wasn't entirely clear on when to swap IIRC, but the idea makes
sense in most cases at least, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c |   28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4d3aca0..8a36ab9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3187,10 +3187,14 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  
>  	p->active = intel_crtc->active;
>  	if (p->active) {
> +		const struct intel_plane_state *plane_state;
> +		int src_w, src_h;
> +
>  		p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
>  		p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
>  
>  		fb = crtc->primary->state->fb;
> +		plane_state = to_intel_plane_state(crtc->primary->state);
>  		/* For planar: Bpp is for uv plane, y_Bpp is for y plane */
>  		if (fb) {
>  			p->plane[0].enabled = true;
> @@ -3205,8 +3209,17 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  			p->plane[0].y_bytes_per_pixel = 0;
>  			p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
>  		}
> -		p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
> -		p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
> +
> +		src_w = drm_rect_width(&plane_state->src) >> 16;
> +		src_h = drm_rect_height(&plane_state->src) >> 16;
> +
> +		if (intel_rotation_90_or_270(crtc->primary->state->rotation)) {
> +			p->plane[0].horiz_pixels = src_h;
> +			p->plane[0].vert_pixels = src_w;
> +		} else {
> +			p->plane[0].horiz_pixels = src_w;
> +			p->plane[0].vert_pixels = src_h;
> +		}
>  		p->plane[0].rotation = crtc->primary->state->rotation;
>  
>  		fb = crtc->cursor->state->fb;
> @@ -3740,8 +3753,15 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	intel_plane->wm.enabled = enabled;
>  	intel_plane->wm.scaled = scaled;
> -	intel_plane->wm.horiz_pixels = sprite_width;
> -	intel_plane->wm.vert_pixels = sprite_height;
> +
> +	if (intel_rotation_90_or_270(plane->state->rotation)) {
> +		intel_plane->wm.horiz_pixels = sprite_height;
> +		intel_plane->wm.vert_pixels = sprite_width;
> +	} else {
> +		intel_plane->wm.horiz_pixels = sprite_width;
> +		intel_plane->wm.vert_pixels = sprite_height;
> +	}
> +
>  	intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
>  
>  	/* For planar: Bpp is for UV plane, y_Bpp is for Y plane */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/15] drm/i915: Set scaler mode for NV12

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:32:59PM -0700, Chandra Konduru wrote:
> 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:
> - new reg defines squashed into patches used them (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_reg.h     |    1 +
>  drivers/gpu/drm/i915/intel_atomic.c |    5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1fa0554..825d721 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5498,6 +5498,7 @@ enum skl_disp_power_wells {
>  #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 9336e80..fd3972c 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -247,7 +247,10 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
>  		}
>  
>  		/* set scaler mode */
> -		if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
> +		if (plane_state && plane_state->base.fb &&
> +			plane_state->base.fb->pixel_format == DRM_FORMAT_NV12) {
> +			scaler_state->scalers[*scaler_id].mode = PS_SCALER_MODE_NV12;
> +		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
>  			/*
>  			 * when only 1 scaler is in use on either pipe A or B,
>  			 * scaler 0 operates in high quality (HQ) mode.
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/15] drm/i915: Update format_is_yuv() to include NV12

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:33:01PM -0700, Chandra Konduru wrote:
> This patch adds NV12 to format_is_yuv() function
> and made it available for both primary and sprite
> planes.
> 
> v2:
> -Use intel_ prefix for format_is_yuv (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h    |    1 +
>  drivers/gpu/drm/i915/intel_sprite.c |    9 +++++----
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f44941b..18632a4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1394,6 +1394,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  void intel_pipe_update_start(struct intel_crtc *crtc,
>  			     uint32_t *start_vbl_count);
>  void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count);
> +bool intel_format_is_yuv(uint32_t format);
>  
>  /* intel_tv.c */
>  void intel_tv_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c13c529..8b73bb8 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -39,14 +39,15 @@
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  
> -static bool
> -format_is_yuv(uint32_t format)
> +bool
> +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;
> @@ -293,7 +294,7 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
>  	int plane = intel_plane->plane;
>  
>  	/* Seems RGB data bypasses the CSC always */
> -	if (!format_is_yuv(format))
> +	if (!intel_format_is_yuv(format))
>  		return;
>  
>  	/*
> @@ -857,7 +858,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  		src_y = src->y1 >> 16;
>  		src_h = drm_rect_height(src) >> 16;
>  
> -		if (format_is_yuv(fb->pixel_format)) {
> +		if (intel_format_is_yuv(fb->pixel_format)) {
>  			src_x &= ~1;
>  			src_w &= ~1;
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/15] drm/i915: Upscale scaler max scale for NV12.

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:33:02PM -0700, Chandra Konduru wrote:
> This patch updates max supported scaler limits for NV12.
> 
> v2:
> -Rebased to current kernel version 4.2.0.rc4 (me)
> 
> v3:
> -simplified max_scale calculation (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   17 +++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h     |    3 ++-
>  drivers/gpu/drm/i915/intel_sprite.c  |    2 +-
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8869779..3f9111e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13423,7 +13423,9 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
>  }
>  
>  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)

Indent fail, but otherwise looks good so

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  {
>  	int max_scale;
>  	struct drm_device *dev;
> @@ -13443,11 +13445,17 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
>  
>  	/*
>  	 * skl max scale is lower of:
> -	 *    close to 3 but not 3, -1 is for that purpose
> +	 *    close to 2 or 3 (NV12: 2, other formats: 3) but not equal,
> +	 *          -1 is for that purpose
>  	 *            or
>  	 *    cdclk/crtc_clock
>  	 */
> -	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
> +	if (pixel_format == DRM_FORMAT_NV12)
> +		max_scale = (2 << 16) - 1;
> +	else
> +		max_scale = (3 << 16) - 1;
> +
> +	max_scale = min(max_scale, (1 << 8) * ((cdclk << 8) / crtc_clock));
>  
>  	return max_scale;
>  }
> @@ -13467,7 +13475,8 @@ intel_check_primary_plane(struct drm_plane *plane,
>  	if (INTEL_INFO(plane->dev)->gen >= 9 &&
>  	    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,
> +				fb ? fb->pixel_format : 0);
>  		can_position = true;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 18632a4..d50b8cb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1140,7 +1140,8 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
>  void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
>  
>  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);
>  
>  unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
>  				     struct drm_i915_gem_object *obj,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 8b73bb8..66d60ae 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -780,7 +780,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  		if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
>  			can_scale = 1;
>  			min_scale = 1;
> -			max_scale = skl_max_scale(intel_crtc, crtc_state);
> +			max_scale = skl_max_scale(intel_crtc, crtc_state, fb->pixel_format);
>  		} else {
>  			can_scale = 0;
>  			min_scale = DRM_PLANE_HELPER_NO_SCALING;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:33:04PM -0700, Chandra Konduru wrote:
> This patch adds NV12 to list of supported formats for
> sprite plane.
> 
> v2:
> - made supported format list const, fixed a leftover -1. (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_sprite.c |   25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 66d60ae..797594e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1029,7 +1029,7 @@ static const uint32_t vlv_plane_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> -static uint32_t skl_plane_formats[] = {
> +static const uint32_t skl_plane_formats[] = {
>  	DRM_FORMAT_RGB565,
>  	DRM_FORMAT_ABGR8888,
>  	DRM_FORMAT_ARGB8888,
> @@ -1041,6 +1041,19 @@ static uint32_t skl_plane_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> +static const uint32_t skl_plane_formats_with_nv12[] = {
> +	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,
> +};
> +
>  int
>  intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  {
> @@ -1112,8 +1125,14 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
>  		intel_plane->disable_plane = skl_disable_plane;
>  		state->scaler_id = -1;
>  
> -		plane_formats = skl_plane_formats;
> -		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> +		if ((pipe == PIPE_A || pipe == PIPE_B) && (plane == 0)) {
> +			plane_formats = skl_plane_formats_with_nv12;
> +			num_plane_formats = ARRAY_SIZE(skl_plane_formats_with_nv12);
> +		} else {
> +			plane_formats = skl_plane_formats;
> +			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> +		}
> +
>  		break;
>  	default:
>  		kfree(intel_plane);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 12/15] drm/i915: Set initial phase & trip for NV12 scaler

[Ville Syrjälä]

On Fri, Sep 04, 2015 at 07:33:08PM -0700, Chandra Konduru wrote:
> This patch sets default initial phase and trip to scale NV12
> content. In future, if needed these can be set via properties
> or other means depending on incoming stream request. Until then
> defaults are fine.
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    7 +++++++
>  drivers/gpu/drm/i915/intel_sprite.c  |    7 +++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6714066..3296d16 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3098,6 +3098,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	int scaler_id = -1;
>  	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
>  	u32 tile_row_adjustment = 0;
> +	u32 hphase = 0, vphase = 0;
>  
>  	plane_state = to_intel_plane_state(plane->state);
>  
> @@ -3181,6 +3182,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  			/* For tile-Yf, uv-subplane tile width is 2x of Y-subplane */
>  			aux_stride = fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED ?
>  				DIV_ROUND_UP(stride, 2) : stride;
> +
> +			hphase = 0x00010001;  /* use trip for both Y and UV */
> +			vphase = 0x00012000;  /* use trip for Y and phase 0.5 for UV */

I don't really know where this "trip" name comes from, but looking at the
spec trip==0 basically seems to mean that the actual initial phase is
the programmed value - 1.

So based on that we are programming 0 for horiz, 0 for Y vertical, and
-0.5 for UV vertical. The spec fails to explain what units these are,
nor is there any information what phase 0.0 corresponds to (center of
the pixel, or left/top edge?). But assuming the units are pixels in the
specific plane in question, and that 0.0 indicates the center, we would 
have the following chroma siting with -0.5:

o=luma sample, x=chroma sample, #=both h+v co-sited

 o o o o
        
 # o # o
        
 o o o o
        
 # o # o

which doesn't really match anything known. Now, assuming we want to use
the MPEG2 chroma siting, it should look like this:

 o o o o
 x   x  
 o o o o
        
 0 0 0 0
 x   x  
 0 0 0 0

So we would want the UV vertical initial phase to be -0.25, which I
believe would mean vphase = 0x00013000

But if I'm wrong and these are always specified in units of luma
pixels, then the -0.5 would be correct.

>  		}
>  	}
>  	plane_offset = y_offset << 16 | x_offset;
> @@ -3209,6 +3213,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) | dst_x);
>  	}
>  
> +	I915_WRITE(SKL_PS_HPHASE(pipe, scaler_id), hphase);
> +	I915_WRITE(SKL_PS_VPHASE(pipe, scaler_id), vphase);
> +
>  	I915_WRITE(PLANE_SURF(pipe, 0), surf_addr);
>  
>  	POSTING_READ(PLANE_SURF(pipe, 0));
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 347fb1f..5ca62b6 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -190,6 +190,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	int scaler_id;
>  	u32 aux_dist = 0, aux_x_offset = 0, aux_y_offset = 0, aux_stride = 0;
>  	u32 tile_row_adjustment = 0;
> +	u32 hphase = 0, vphase = 0;
>  
>  	plane_ctl = PLANE_CTL_ENABLE |
>  		PLANE_CTL_PIPE_CSC_ENABLE;
> @@ -264,6 +265,9 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  			/* For tile-Yf, uv-subplane tile width is 2x of Y-subplane */
>  			aux_stride = fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED ?
>  				DIV_ROUND_UP(stride, 2) : stride;
> +
> +			hphase = 0x00010001;  /* use trip for both Y and UV */
> +			vphase = 0x00012000;  /* use trip for Y and phase 0.5 for UV */
>  		}
>  	}
>  	plane_offset = y_offset << 16 | x_offset;
> @@ -292,6 +296,9 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  		I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) | crtc_x);
>  	}
>  
> +	I915_WRITE(SKL_PS_HPHASE(pipe, scaler_id), hphase);
> +	I915_WRITE(SKL_PS_VPHASE(pipe, scaler_id), vphase);
> +
>  	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
>  	I915_WRITE(PLANE_SURF(pipe, plane), surf_addr);
>  	POSTING_READ(PLANE_SURF(pipe, plane));
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Fri, Sep 11, 2015 at 09:43:48AM -0700, Chandra Konduru wrote:
> This patch adds NV12 to list of supported formats for
> primary plane.
> 
> v2:
> -Rebased (me)
> 
> v3:
> -Rebased on top of primary plane YUV support patch (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12

Was wondering where this went., but it was just hiding (no v2 in patch
subject).

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.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 4122359..2a5170e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -78,6 +78,23 @@ static const uint32_t skl_primary_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> +/* Primary plane formats for gen >= 9 with NV12 */
> +static const uint32_t skl_primary_formats_with_nv12[] = {
> +	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,
> @@ -13611,8 +13628,13 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  		primary->plane = !pipe;
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
> -		intel_primary_formats = skl_primary_formats;
> -		num_formats = ARRAY_SIZE(skl_primary_formats);
> +		if (pipe == PIPE_A || pipe == PIPE_B) {
> +			intel_primary_formats = skl_primary_formats_with_nv12;
> +			num_formats = ARRAY_SIZE(skl_primary_formats_with_nv12);
> +		} else {
> +			intel_primary_formats = skl_primary_formats;
> +			num_formats = ARRAY_SIZE(skl_primary_formats);
> +		}
>  	} else if (INTEL_INFO(dev)->gen >= 4) {
>  		intel_primary_formats = i965_primary_formats;
>  		num_formats = ARRAY_SIZE(i965_primary_formats);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Ville Syrjälä]

On Fri, Sep 11, 2015 at 09:44:18AM -0700, Chandra Konduru wrote:
> This patch adds NV12 as supported format to
> intel_framebuffer_init and performs various checks.
> 
> v2:
> -Fix an issue in checks added (me)
> 
> v3:
> -cosmetic update, split checks into two (Ville)
> 
> v4:
> -Add stride alignment and modifier checks for UV subplane (Ville)
> 
> v5:
> -Make modifier check general (Ville)
> -Check tile-y uv start alignment from begining of page (Ville)
> 
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12
> ---
>  drivers/gpu/drm/i915/intel_display.c |   66 +++++++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h     |    2 +-
>  drivers/gpu/drm/i915/intel_sprite.c  |    2 +-
>  3 files changed, 55 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2a5170e..af28ca9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2914,9 +2914,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  }
>  
>  u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> -			      uint32_t pixel_format)
> +			      uint32_t pixel_format, int plane)
>  {
> -	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
> +	u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, plane) * 8;
>  
>  	/*
>  	 * The stride is either expressed as a multiple of 64 bytes
> @@ -3125,7 +3125,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  
>  	obj = intel_fb_obj(fb);
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> -					       fb->pixel_format);
> +					       fb->pixel_format, 0);
>  	surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
>  
>  	/*
> @@ -9104,7 +9104,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>  
>  	val = I915_READ(PLANE_STRIDE(pipe, 0));
>  	stride_mult = intel_fb_stride_alignment(dev, fb->modifier[0],
> -						fb->pixel_format);
> +						fb->pixel_format, 0);
>  	fb->pitches[0] = (val & 0x3ff) * stride_mult;
>  
>  	aligned_height = intel_fb_align_height(dev, fb->height,
> @@ -11175,7 +11175,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	 */
>  	stride = fb->pitches[0] /
>  		 intel_fb_stride_alignment(dev, fb->modifier[0],
> -					   fb->pixel_format);
> +					   fb->pixel_format, 0);
>  
>  	/*
>  	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  {
>  	unsigned int aligned_height;
>  	int ret;
> +	int i;
>  	u32 pitch_limit, stride_alignment;
>  
>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  		}
>  	} else {
>  		if (obj->tiling_mode == I915_TILING_X)
> -			mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
> +			for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++)
> +				mode_cmd->modifier[i] = I915_FORMAT_MOD_X_TILED;

The other branch needs updating too so that it will reject the operation
if the modifier disagrees with the obj tiling mode.

>  		else if (obj->tiling_mode == I915_TILING_Y) {
>  			DRM_DEBUG("No Y tiling for legacy addfb\n");
>  			return -EINVAL;
> @@ -14280,12 +14282,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  		return -EINVAL;
>  	}
>  
> -	stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
> -						     mode_cmd->pixel_format);
> -	if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
> -		DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
> -			  mode_cmd->pitches[0], stride_alignment);
> -		return -EINVAL;
> +	/* check stride alignment for sub-planes */
> +	for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
> +		stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[i],
> +						     mode_cmd->pixel_format, i);
> +		if (mode_cmd->pitches[i] & (stride_alignment - 1)) {
> +			DRM_DEBUG("subplane %d pitch (%d) must be at least %u bytes "
> +				"aligned\n", i, mode_cmd->pitches[i], stride_alignment);
> +			return -EINVAL;
> +		}
>  	}
>  
>  	pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
> @@ -14352,9 +14357,44 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  			return -EINVAL;
>  		}
>  		break;
> +	case DRM_FORMAT_NV12:
> +		if (INTEL_INFO(dev)->gen < 9) {
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +				drm_get_format_name(mode_cmd->pixel_format));
> +			return -EINVAL;
> +		}
> +		if (!mode_cmd->offsets[1]) {
> +			DRM_DEBUG("uv start offset not set\n");
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> +			DRM_DEBUG("y and uv subplanes have different pitches\n");
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->handles[0] != mode_cmd->handles[1]) {
> +			DRM_DEBUG("y and uv subplanes have different handles\n");
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->modifier[0] != mode_cmd->modifier[1]) {
> +			DRM_DEBUG("y and uv subplanes have different modifiers\n");
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
> +			(mode_cmd->offsets[1] & 0xFFF)) {

I've been trying to solicit ideas on how we should define the offsets[];
raw byte offset, or linear offset. I didn't get many opinions yet. So we
need to figure it out and document it somewhere before we expose it to
the world. In the meantime we could just reject non tile row aligned
offsets regardless of the tiling mode.

> +			DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new tile-row\n",
> +				mode_cmd->offsets[1]);
> +			return -EINVAL;
> +		}
> +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
> +			(((mode_cmd->offsets[1] & 0xFFF) / mode_cmd->pitches[1]) % 4)) {
> +			DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line aligned\n",
> +				mode_cmd->offsets[1]);
> +			return -EINVAL;
> +		}
> +		break;
>  	default:
>  		DRM_DEBUG("unsupported pixel format: %s\n",
> -			  drm_get_format_name(mode_cmd->pixel_format));
> +			drm_get_format_name(mode_cmd->pixel_format));
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d50b8cb..62d2a11 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -980,7 +980,7 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
>  void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
>  			enum fb_op_origin origin);
>  u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> -			      uint32_t pixel_format);
> +			      uint32_t pixel_format, int plane);
>  
>  /* intel_audio.c */
>  void intel_init_audio(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 797594e..49feae0 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -203,7 +203,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  				       src_w != crtc_w || src_h != crtc_h);
>  
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> -					       fb->pixel_format);
> +					       fb->pixel_format, 0);
>  
>  	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Tue, Sep 29, 2015 at 08:50:20PM +0300, Ville Syrjälä wrote:
> On Fri, Sep 04, 2015 at 07:33:04PM -0700, Chandra Konduru wrote:
> > This patch adds NV12 to list of supported formats for
> > sprite plane.
> > 
> > v2:
> > - made supported format list const, fixed a leftover -1. (Ville)
> > 
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > Testcase: igt/kms_nv12
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Oh and the ordering of patches doesn't seem very good here. This and the
primary plane equvalent need to be some of the last patches in the
series so that we don't expose the format before we're ready to handle
it.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c |   25 ++++++++++++++++++++++---
> >  1 file changed, 22 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 66d60ae..797594e 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -1029,7 +1029,7 @@ static const uint32_t vlv_plane_formats[] = {
> >  	DRM_FORMAT_VYUY,
> >  };
> >  
> > -static uint32_t skl_plane_formats[] = {
> > +static const uint32_t skl_plane_formats[] = {
> >  	DRM_FORMAT_RGB565,
> >  	DRM_FORMAT_ABGR8888,
> >  	DRM_FORMAT_ARGB8888,
> > @@ -1041,6 +1041,19 @@ static uint32_t skl_plane_formats[] = {
> >  	DRM_FORMAT_VYUY,
> >  };
> >  
> > +static const uint32_t skl_plane_formats_with_nv12[] = {
> > +	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,
> > +};
> > +
> >  int
> >  intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> >  {
> > @@ -1112,8 +1125,14 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
> >  		intel_plane->disable_plane = skl_disable_plane;
> >  		state->scaler_id = -1;
> >  
> > -		plane_formats = skl_plane_formats;
> > -		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> > +		if ((pipe == PIPE_A || pipe == PIPE_B) && (plane == 0)) {
> > +			plane_formats = skl_plane_formats_with_nv12;
> > +			num_plane_formats = ARRAY_SIZE(skl_plane_formats_with_nv12);
> > +		} else {
> > +			plane_formats = skl_plane_formats;
> > +			num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> > +		}
> > +
> >  		break;
> >  	default:
> >  		kfree(intel_plane);
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec

[Daniel Vetter]

On Tue, Sep 29, 2015 at 08:45:05PM +0300, Ville Syrjälä wrote:
> On Fri, Sep 04, 2015 at 07:32:57PM -0700, Chandra Konduru wrote:
> > Properly allocate min blocks per hw requirements.
> > 
> > v2:
> > - changed helper functional param to bool, some code simplification (Ville)
> > 
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> 
> IIRC I gave my r-b alrady?

Yup, it was a conditional r-b ("fix this tiny thing and you have the r-b")
somewhen in August.

Chandra in that case please add the r-b yourself to avoid wasting people's
time. Also, not doing that just increases the changes that someone will
spot something new ;-)
-Daniel


> 
> But I'll toss it in again:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c |   29 +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index fff0c22..4d3aca0 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2959,6 +2959,31 @@ skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
> >  	return total_data_rate;
> >  }
> >  
> > +static uint16_t
> > +skl_dbuf_min_alloc(const struct intel_plane_wm_parameters *p, bool y_plane)
> > +{
> > +	uint16_t min_alloc;
> > +
> > +	/* For packed formats, no y-plane, return 0 */
> > +	if (y_plane && !p->y_bytes_per_pixel)
> > +		return 0;
> > +
> > +	if (p->tiling == I915_FORMAT_MOD_Y_TILED ||
> > +	    p->tiling == I915_FORMAT_MOD_Yf_TILED) {
> > +		uint32_t min_scanlines = 8;
> > +		uint8_t bytes_per_pixel =
> > +			y_plane ? p->y_bytes_per_pixel : p->bytes_per_pixel;
> > +
> > +		min_scanlines = 32 / bytes_per_pixel;
> > +		min_alloc = DIV_ROUND_UP((4 * p->horiz_pixels/(y_plane ? 1 : 2) *
> > +			bytes_per_pixel), 512) * min_scanlines/4 + 3;
> > +	} else {
> > +		min_alloc = 8;
> > +	}
> > +
> > +	return min_alloc;
> > +}
> > +
> >  static void
> >  skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> >  		      const struct intel_wm_config *config,
> > @@ -2999,9 +3024,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> >  		if (!p->enabled)
> >  			continue;
> >  
> > -		minimum[plane] = 8;
> > +		minimum[plane] = skl_dbuf_min_alloc(p, false);   /* uv-plane/packed */
> >  		alloc_size -= minimum[plane];
> > -		y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
> > +		y_minimum[plane] = skl_dbuf_min_alloc(p, true);  /* y-plane */
> >  		alloc_size -= y_minimum[plane];
> >  	}
> >  
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/15] drm/i915: Set scaler mode for NV12

[Daniel Vetter]

On Tue, Sep 29, 2015 at 08:47:15PM +0300, Ville Syrjälä wrote:
> On Fri, Sep 04, 2015 at 07:32:59PM -0700, Chandra Konduru wrote:
> > 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:
> > - new reg defines squashed into patches used them (Ville)
> > 
> > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Merged up to this patch to dinq.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h     |    1 +
> >  drivers/gpu/drm/i915/intel_atomic.c |    5 ++++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 1fa0554..825d721 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5498,6 +5498,7 @@ enum skl_disp_power_wells {
> >  #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 9336e80..fd3972c 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -247,7 +247,10 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
> >  		}
> >  
> >  		/* set scaler mode */
> > -		if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
> > +		if (plane_state && plane_state->base.fb &&
> > +			plane_state->base.fb->pixel_format == DRM_FORMAT_NV12) {
> > +			scaler_state->scalers[*scaler_id].mode = PS_SCALER_MODE_NV12;
> > +		} else if (num_scalers_need == 1 && intel_crtc->pipe != PIPE_C) {
> >  			/*
> >  			 * when only 1 scaler is in use on either pipe A or B,
> >  			 * scaler 0 operates in high quality (HQ) mode.
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/15] drm/i915: Set scaler mode for NV12

[Daniel Vetter]

On Wed, Sep 30, 2015 at 02:22:12PM +0200, Daniel Vetter wrote:
> On Tue, Sep 29, 2015 at 08:47:15PM +0300, Ville Syrjälä wrote:
> > On Fri, Sep 04, 2015 at 07:32:59PM -0700, Chandra Konduru wrote:
> > > 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:
> > > - new reg defines squashed into patches used them (Ville)
> > > 
> > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> > 
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Merged up to this patch to dinq.

Ok this super-badly conflicted with the atomic watermarks. Since those are
ready and nv12 still needs more work I dropped these 3 patches again.

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

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Konduru, Chandra]

> > @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct
> drm_device *dev,
> >  {
> >  	unsigned int aligned_height;
> >  	int ret;
> > +	int i;
> >  	u32 pitch_limit, stride_alignment;
> >
> >  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> > @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct
> drm_device *dev,
> >  		}
> >  	} else {
> >  		if (obj->tiling_mode == I915_TILING_X)
> > -			mode_cmd->modifier[0] =
> I915_FORMAT_MOD_X_TILED;
> > +			for (i = 0; i < drm_format_num_planes(mode_cmd-
> >pixel_format); i++)
> > +				mode_cmd->modifier[i] =
> I915_FORMAT_MOD_X_TILED;
> 
> The other branch needs updating too so that it will reject the operation
> if the modifier disagrees with the obj tiling mode.

Is below something you meant?

@@ -14223,10 +14223,12 @@ static int intel_framebuffer_init(struct drm_device *d
        if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
                /* Enforce that fb modifier and tiling mode match, but only for
                 * X-tiled. This is needed for FBC. */
-               if (!!(obj->tiling_mode == I915_TILING_X) !=
-                   !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
-                       DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
-                       return -EINVAL;
+               for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i
+                       if (!!(obj->tiling_mode == I915_TILING_X) !=
+                       !!(mode_cmd->modifier[i] == I915_FORMAT_MOD_X_TILED)) {
+                               DRM_DEBUG("tiling_mode doesn't match fb modifier
+                               return -EINVAL;
+                       }
                }
        } else {

> > +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED
> &&
> > +			(mode_cmd->offsets[1] & 0xFFF)) {
> 
> I've been trying to solicit ideas on how we should define the offsets[];
> raw byte offset, or linear offset. I didn't get many opinions yet. So we
> need to figure it out and document it somewhere before we expose it to
> the world. In the meantime we could just reject non tile row aligned
> offsets regardless of the tiling mode.

Above check is simply making sure tile Yf, uv offset starts on a new page. 
Is there any issue with above check?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Ville Syrjälä]

On Wed, Sep 30, 2015 at 10:58:07PM +0000, Konduru, Chandra wrote:
> > > @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct
> > drm_device *dev,
> > >  {
> > >  	unsigned int aligned_height;
> > >  	int ret;
> > > +	int i;
> > >  	u32 pitch_limit, stride_alignment;
> > >
> > >  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> > > @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct
> > drm_device *dev,
> > >  		}
> > >  	} else {
> > >  		if (obj->tiling_mode == I915_TILING_X)
> > > -			mode_cmd->modifier[0] =
> > I915_FORMAT_MOD_X_TILED;
> > > +			for (i = 0; i < drm_format_num_planes(mode_cmd-
> > >pixel_format); i++)
> > > +				mode_cmd->modifier[i] =
> > I915_FORMAT_MOD_X_TILED;
> > 
> > The other branch needs updating too so that it will reject the operation
> > if the modifier disagrees with the obj tiling mode.
> 
> Is below something you meant?
> 
> @@ -14223,10 +14223,12 @@ static int intel_framebuffer_init(struct drm_device *d
>         if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
>                 /* Enforce that fb modifier and tiling mode match, but only for
>                  * X-tiled. This is needed for FBC. */
> -               if (!!(obj->tiling_mode == I915_TILING_X) !=
> -                   !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
> -                       DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
> -                       return -EINVAL;
> +               for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i
> +                       if (!!(obj->tiling_mode == I915_TILING_X) !=
> +                       !!(mode_cmd->modifier[i] == I915_FORMAT_MOD_X_TILED)) {
> +                               DRM_DEBUG("tiling_mode doesn't match fb modifier
> +                               return -EINVAL;
> +                       }
>                 }

Yep.

>         } else {
> 
> > > +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED
> > &&
> > > +			(mode_cmd->offsets[1] & 0xFFF)) {
> > 
> > I've been trying to solicit ideas on how we should define the offsets[];
> > raw byte offset, or linear offset. I didn't get many opinions yet. So we
> > need to figure it out and document it somewhere before we expose it to
> > the world. In the meantime we could just reject non tile row aligned
> > offsets regardless of the tiling mode.
> 
> Above check is simply making sure tile Yf, uv offset starts on a new page. 
> Is there any issue with above check?

It won't necessarily be a page boundary if we interpret offsets[] as a linear
offset.

Eg. let's assume 4x4 tile size, stride=8, and offset=16. If interpret
the offset as a linear offset we would land at 'x', but interpreted as
a raw byte offset (not sure that's a good name, maybe untiled offset?)
we'd land at 'y'.

-----------
|    |y   |
|    |    |
|x   |    |
|    |    |
-----------
|z   |    |
|    |    |
|    |    |
|    |    |
-----------

If we had offset=32, then of course we would land at 'z' for both cases,
which is why I suggested that if we haven't made up our mind about what
offsets[] is, we could require it to be tile row aligned so that there
would be no difference between the two interpretations.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Ville Syrjälä]

On Thu, Oct 01, 2015 at 02:37:27PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 30, 2015 at 10:58:07PM +0000, Konduru, Chandra wrote:
> > > > @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct
> > > drm_device *dev,
> > > >  {
> > > >  	unsigned int aligned_height;
> > > >  	int ret;
> > > > +	int i;
> > > >  	u32 pitch_limit, stride_alignment;
> > > >
> > > >  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> > > > @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct
> > > drm_device *dev,
> > > >  		}
> > > >  	} else {
> > > >  		if (obj->tiling_mode == I915_TILING_X)
> > > > -			mode_cmd->modifier[0] =
> > > I915_FORMAT_MOD_X_TILED;
> > > > +			for (i = 0; i < drm_format_num_planes(mode_cmd-
> > > >pixel_format); i++)
> > > > +				mode_cmd->modifier[i] =
> > > I915_FORMAT_MOD_X_TILED;
> > > 
> > > The other branch needs updating too so that it will reject the operation
> > > if the modifier disagrees with the obj tiling mode.
> > 
> > Is below something you meant?
> > 
> > @@ -14223,10 +14223,12 @@ static int intel_framebuffer_init(struct drm_device *d
> >         if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> >                 /* Enforce that fb modifier and tiling mode match, but only for
> >                  * X-tiled. This is needed for FBC. */
> > -               if (!!(obj->tiling_mode == I915_TILING_X) !=
> > -                   !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
> > -                       DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
> > -                       return -EINVAL;
> > +               for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i
> > +                       if (!!(obj->tiling_mode == I915_TILING_X) !=
> > +                       !!(mode_cmd->modifier[i] == I915_FORMAT_MOD_X_TILED)) {
> > +                               DRM_DEBUG("tiling_mode doesn't match fb modifier
> > +                               return -EINVAL;
> > +                       }
> >                 }
> 
> Yep.
> 
> >         } else {
> > 
> > > > +		if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED
> > > &&
> > > > +			(mode_cmd->offsets[1] & 0xFFF)) {
> > > 
> > > I've been trying to solicit ideas on how we should define the offsets[];
> > > raw byte offset, or linear offset. I didn't get many opinions yet. So we
> > > need to figure it out and document it somewhere before we expose it to
> > > the world. In the meantime we could just reject non tile row aligned
> > > offsets regardless of the tiling mode.
> > 
> > Above check is simply making sure tile Yf, uv offset starts on a new page. 
> > Is there any issue with above check?
> 
> It won't necessarily be a page boundary if we interpret offsets[] as a linear
> offset.
> 
> Eg. let's assume 4x4 tile size, stride=8, and offset=16. If interpret
> the offset as a linear offset we would land at 'x', but interpreted as
> a raw byte offset (not sure that's a good name, maybe untiled offset?)
> we'd land at 'y'.
> 
> -----------
> |    |y   |
> |    |    |
> |x   |    |
> |    |    |
> -----------
> |z   |    |
> |    |    |
> |    |    |
> |    |    |
> -----------
> 
> If we had offset=32, then of course we would land at 'z' for both cases,
> which is why I suggested that if we haven't made up our mind about what
> offsets[] is, we could require it to be tile row aligned so that there
> would be no difference between the two interpretations.

Oh and I just figured out that linear offset would probably be better
because that also isolates us from having to think about the internal
tile layout, as in which way the bytes/owords/whatver are walked within
the tile.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Konduru, Chandra]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Thursday, October 01, 2015 4:41 AM
> To: Konduru, Chandra
> Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville
> Subject: Re: [Intel-gfx] [PATCH 09/15] drm/i915: Add NV12 support to
> intel_framebuffer_init
> 
> On Thu, Oct 01, 2015 at 02:37:27PM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 30, 2015 at 10:58:07PM +0000, Konduru, Chandra wrote:
> > > > > @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct
> > > > drm_device *dev,
> > > > >  {
> > > > >  	unsigned int aligned_height;
> > > > >  	int ret;
> > > > > +	int i;
> > > > >  	u32 pitch_limit, stride_alignment;
> > > > >
> > > > >  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> > > > > @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct
> > > > drm_device *dev,
> > > > >  		}
> > > > >  	} else {
> > > > >  		if (obj->tiling_mode == I915_TILING_X)
> > > > > -			mode_cmd->modifier[0] =
> > > > I915_FORMAT_MOD_X_TILED;
> > > > > +			for (i = 0; i <
> drm_format_num_planes(mode_cmd-
> > > > >pixel_format); i++)
> > > > > +				mode_cmd->modifier[i] =
> > > > I915_FORMAT_MOD_X_TILED;
> > > >
> > > > The other branch needs updating too so that it will reject the operation
> > > > if the modifier disagrees with the obj tiling mode.
> > >
> > > Is below something you meant?
> > >
> > > @@ -14223,10 +14223,12 @@ static int intel_framebuffer_init(struct
> drm_device *d
> > >         if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
> > >                 /* Enforce that fb modifier and tiling mode match, but only for
> > >                  * X-tiled. This is needed for FBC. */
> > > -               if (!!(obj->tiling_mode == I915_TILING_X) !=
> > > -                   !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
> > > -                       DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
> > > -                       return -EINVAL;
> > > +               for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i
> > > +                       if (!!(obj->tiling_mode == I915_TILING_X) !=
> > > +                       !!(mode_cmd->modifier[i] == I915_FORMAT_MOD_X_TILED)) {
> > > +                               DRM_DEBUG("tiling_mode doesn't match fb modifier
> > > +                               return -EINVAL;
> > > +                       }
> > >                 }
> >
> > Yep.
> >
> > >         } else {
> > >
> > > > > +		if (mode_cmd->modifier[1] ==
> I915_FORMAT_MOD_Yf_TILED
> > > > &&
> > > > > +			(mode_cmd->offsets[1] & 0xFFF)) {
> > > >
> > > > I've been trying to solicit ideas on how we should define the offsets[];
> > > > raw byte offset, or linear offset. I didn't get many opinions yet. So we
> > > > need to figure it out and document it somewhere before we expose it to
> > > > the world. In the meantime we could just reject non tile row aligned
> > > > offsets regardless of the tiling mode.
> > >
> > > Above check is simply making sure tile Yf, uv offset starts on a new page.
> > > Is there any issue with above check?
> >
> > It won't necessarily be a page boundary if we interpret offsets[] as a linear
> > offset.
> >
> > Eg. let's assume 4x4 tile size, stride=8, and offset=16. If interpret
> > the offset as a linear offset we would land at 'x', but interpreted as
> > a raw byte offset (not sure that's a good name, maybe untiled offset?)
> > we'd land at 'y'.
> >
> > -----------
> > |    |y   |
> > |    |    |
> > |x   |    |
> > |    |    |
> > -----------
> > |z   |    |
> > |    |    |
> > |    |    |
> > |    |    |
> > -----------
> >
> > If we had offset=32, then of course we would land at 'z' for both cases,
> > which is why I suggested that if we haven't made up our mind about what
> > offsets[] is, we could require it to be tile row aligned so that there
> > would be no difference between the two interpretations.
> 
> Oh and I just figured out that linear offset would probably be better
> because that also isolates us from having to think about the internal
> tile layout, as in which way the bytes/owords/whatver are walked within
> the tile.

What is the issue with current offsets the patch is using?
Also I think linear offset doesn't work for hw (it is actually confusing to say
linear offset for a tiling format to start with). Requires a conversion to tiling format
which requires i915 addfb logic to know tiling limitations/restrictions from media 
and/or other engines. 
Userlands know where the UV start is for every tiling format if they are using the
tiling format and giving the UV offset to i915. If someone didn't set offset right, 
above check is helping to point that out.


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