* [PATCH 00/15] drm/i915: Adding NV12 for skylake display
@ 2015-08-20 1:02 Chandra Konduru
2015-08-20 1:02 ` [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec Chandra Konduru
` (14 more replies)
0 siblings, 15 replies; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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.
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: Add register definitions for NV12 support
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 workarounds
drm/i915: Add 90/270 rotation for NV12 format.
drivers/gpu/drm/i915/i915_reg.h | 47 +++++++++
drivers/gpu/drm/i915/intel_atomic.c | 5 +-
drivers/gpu/drm/i915/intel_csr.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 178 +++++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/intel_drv.h | 5 +-
drivers/gpu/drm/i915/intel_pm.c | 71 ++++++++++++--
drivers/gpu/drm/i915/intel_sprite.c | 121 +++++++++++++++++++----
7 files changed, 387 insertions(+), 42 deletions(-)
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 8:17 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h Chandra Konduru
` (13 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
Properly allocate min blocks per hw requirements.
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fff0c22..da3046f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2959,6 +2959,41 @@ 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, int 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;
+
+ switch (bytes_per_pixel) {
+ case 1:
+ min_scanlines = 32;
+ break;
+ case 2:
+ min_scanlines = 16;
+ break;
+ case 8:
+ WARN(1, "Unsupported pixel depth for rotation");
+ }
+ 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 +3034,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
if (!p->enabled)
continue;
- minimum[plane] = 8;
+ minimum[plane] = skl_dbuf_min_alloc(p, 0); /* 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, 1); /* 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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
2015-08-20 1:02 ` [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 8:31 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 03/15] drm/i915: Add register definitions for NV12 support Chandra Konduru
` (12 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
This patch swaps src width and height for dbuf/wm calculations
when rotation is 90/270 as per hw requirements.
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index da3046f..c455946 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3193,6 +3193,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
enum pipe pipe = intel_crtc->pipe;
struct drm_plane *plane;
struct drm_framebuffer *fb;
+ struct intel_plane_state *plane_state;
+ int src_w, src_h;
int i = 1; /* Index for sprite planes start */
p->active = intel_crtc->active;
@@ -3201,6 +3203,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
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;
@@ -3215,8 +3218,22 @@ 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;
+
+ if (drm_rect_width(&plane_state->src)) {
+ src_w = drm_rect_width(&plane_state->src) >> 16;
+ src_h = drm_rect_height(&plane_state->src) >> 16;
+ } else {
+ src_w = intel_crtc->config->pipe_src_w;
+ src_h = intel_crtc->config->pipe_src_h;
+ }
+
+ 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;
@@ -3750,8 +3767,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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 03/15] drm/i915: Add register definitions for NV12 support
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
2015-08-20 1:02 ` [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec Chandra Konduru
2015-08-20 1:02 ` [PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 8:40 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 04/15] drm/i915: Set scaler mode for NV12 Chandra Konduru
` (11 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
This patch adds register definitions for skylake
display NV12 support.
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1fa0554..c4d732f 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)
@@ -5601,6 +5602,32 @@ 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
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 04/15] drm/i915: Set scaler mode for NV12
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (2 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 03/15] drm/i915: Add register definitions for NV12 support Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 8:53 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 05/15] drm/i915: Stage scaler request for NV12 as src format Chandra Konduru
` (10 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
This patch sets appropriate scaler mode for NV12 format.
In this mode, skylake scaler does either chroma-upsampling or
chroma-upsampling and resolution scaling.
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
drivers/gpu/drm/i915/intel_atomic.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 05/15] drm/i915: Stage scaler request for NV12 as src format
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (3 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 04/15] drm/i915: Set scaler mode for NV12 Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 10:17 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 06/15] drm/i915: Update format_is_yuv() to include NV12 Chandra Konduru
` (9 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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)
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3ee1c17..411b211 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4341,10 +4341,27 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
}
}
+static int skl_need_scaling(int src_w, int dst_w, int src_h, int dst_h,
+ int rotation, uint32_t pixel_format)
+{
+ /* scaling is required when src dst sizes doesn't match or format is NV12 */
+ if (src_w != dst_w || src_h != dst_h)
+ return 1;
+
+ if (intel_rotation_90_or_270(rotation) &&
+ (src_h != dst_w || src_w != dst_h))
+ return 1;
+
+ if (pixel_format == DRM_FORMAT_NV12)
+ return 1;
+
+ return 0;
+}
+
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;
@@ -4352,9 +4369,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
to_intel_crtc(crtc_state->base.crtc);
int 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, dst_w, src_h, dst_h, rotation,
+ pixel_format);
/*
* if plane is being disabled or scaler is no more required or force detach
@@ -4423,7 +4439,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 +4475,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 +4501,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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 06/15] drm/i915: Update format_is_yuv() to include NV12
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (4 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 05/15] drm/i915: Stage scaler request for NV12 as src format Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 10:17 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 07/15] drm/i915: Upscale scaler max scale for NV12 Chandra Konduru
` (8 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 07/15] drm/i915: Upscale scaler max scale for NV12.
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (5 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 06/15] drm/i915: Update format_is_yuv() to include NV12 Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 10:22 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 08/15] drm/i915: Add NV12 as supported format for primary plane Chandra Konduru
` (7 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
This patch updates max supported scaler limits for NV12.
v2:
-Rebased to current kernel version 4.2.0.rc4 (me)
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 13 +++++++++----
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 411b211..b1d9edf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13421,7 +13421,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;
@@ -13441,11 +13443,13 @@ 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));
+ max_scale = min((1 << 16) * (pixel_format == DRM_FORMAT_NV12 ? 2 : 3) - 1,
+ (1 << 8) * ((cdclk << 8) / crtc_clock));
return max_scale;
}
@@ -13465,7 +13469,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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 08/15] drm/i915: Add NV12 as supported format for primary plane
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (6 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 07/15] drm/i915: Upscale scaler max scale for NV12 Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-08-26 8:40 ` Daniel Vetter
2015-08-20 1:02 ` [PATCH 09/15] drm/i915: Add NV12 as supported format for sprite plane Chandra Konduru
` (6 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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 b1d9edf..e4a6a91 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,
@@ -13606,8 +13619,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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 09/15] drm/i915: Add NV12 as supported format for sprite plane
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (7 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 08/15] drm/i915: Add NV12 as supported format for primary plane Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 10:28 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init Chandra Konduru
` (5 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
This patch adds NV12 to list of supported formats for
sprite plane.
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
drivers/gpu/drm/i915/intel_sprite.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 66d60ae..4e8c020 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1041,6 +1041,19 @@ static uint32_t skl_plane_formats[] = {
DRM_FORMAT_VYUY,
};
+static 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) - 1;
+ }
+
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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (8 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 09/15] drm/i915: Add NV12 as supported format for sprite plane Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 10:40 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 11/15] drm/i915: Add NV12 to primary plane programming Chandra Konduru
` (4 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
This patch adds NV12 as supported format to
intel_framebuffer_init and performs various checks.
v2:
-Fix an issue in checks added (me)
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e4a6a91..4df4d77 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14343,6 +14343,34 @@ 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] ||
+ mode_cmd->handles[0] != mode_cmd->handles[1]) {
+ DRM_DEBUG("y and uv subplanes have different parameters\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));
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (9 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 11:09 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 12/15] drm/i915: Add NV12 to sprite " Chandra Konduru
` (3 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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)
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
Testcase: igt/kms_nv12
---
drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4df4d77..329651e 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 ?
+ 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);
}
@@ -11626,6 +11657,12 @@ 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;
+ /* Adjust (macro)pixel boundary */
+ if (fb && intel_format_is_yuv(fb->pixel_format)) {
+ to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
+ to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
+ }
+
if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
plane->type != DRM_PLANE_TYPE_CURSOR) {
ret = skl_update_scaler_plane(
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 12/15] drm/i915: Add NV12 to sprite plane programming.
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (10 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 11/15] drm/i915: Add NV12 to primary plane programming Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-08-20 1:02 ` [PATCH 13/15] drm/i915: Set initial phase & trip for NV12 scaler Chandra Konduru
` (2 subsequent siblings)
14 siblings, 0 replies; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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.
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 4e8c020..a1384a7 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 ?
+ 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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 13/15] drm/i915: Set initial phase & trip for NV12 scaler
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (11 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 12/15] drm/i915: Add NV12 to sprite " Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 11:15 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 14/15] drm/i915: skl nv12 workarounds Chandra Konduru
2015-08-20 1:02 ` [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format Chandra Konduru
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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 329651e..419660d 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 ?
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 a1384a7..0ea9273 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 ?
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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (12 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 13/15] drm/i915: Set initial phase & trip for NV12 scaler Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-08-26 8:42 ` Daniel Vetter
2015-09-04 11:26 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format Chandra Konduru
14 siblings, 2 replies; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
Adding driver workarounds for nv12.
Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
drivers/gpu/drm/i915/intel_csr.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 7 +++++++
5 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c4d732f..3192837 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5354,6 +5354,26 @@ 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)
+
+#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_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index ba1ae03..559a7f5 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -181,7 +181,7 @@ static const struct stepping_info skl_stepping_info[] = {
{'G', '0'}, {'H', '0'}, {'I', '0'}
};
-static char intel_get_stepping(struct drm_device *dev)
+char intel_get_stepping(struct drm_device *dev)
{
if (IS_SKYLAKE(dev) && (dev->pdev->revision <
ARRAY_SIZE(skl_stepping_info)))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 419660d..2158b8f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3196,6 +3196,16 @@ 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);
+ DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
+ IS_SKYLAKE(dev), IS_BROXTON(dev));
+
+ 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;
@@ -5004,6 +5014,21 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
}
+
+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;
@@ -5094,6 +5119,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)
@@ -5211,6 +5239,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)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d50b8cb..63750d5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1158,6 +1158,7 @@ void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
enum csr_state state);
void intel_csr_load_program(struct drm_device *dev);
void intel_csr_ucode_fini(struct drm_device *dev);
+char intel_get_stepping(struct drm_device *dev);
void assert_csr_loaded(struct drm_i915_private *dev_priv);
/* intel_dp.c */
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0ea9273..9d1c5b9 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -278,6 +278,13 @@ 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);
+ 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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format.
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
` (13 preceding siblings ...)
2015-08-20 1:02 ` [PATCH 14/15] drm/i915: skl nv12 workarounds Chandra Konduru
@ 2015-08-20 1:02 ` Chandra Konduru
2015-09-04 11:30 ` Ville Syrjälä
14 siblings, 1 reply; 57+ messages in thread
From: Chandra Konduru @ 2015-08-20 1:02 UTC (permalink / raw
To: intel-gfx; +Cc: daniel.vetter, ville.syrjala
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 | 28 +++++++++++------
drivers/gpu/drm/i915/intel_sprite.c | 56 ++++++++++++++++++++++------------
2 files changed, 56 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2158b8f..19d0f8b 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;
@@ -11697,8 +11702,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
/* Adjust (macro)pixel boundary */
if (fb && intel_format_is_yuv(fb->pixel_format)) {
- to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
- to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
+ if (intel_rotation_90_or_270(plane_state->rotation)) {
+ to_intel_plane_state(plane_state)->src.y1 &= ~0x10000;
+ to_intel_plane_state(plane_state)->src.y2 &= ~0x10000;
+ } else {
+ to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
+ to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
+ }
}
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 9d1c5b9..3522cb0 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;
@@ -900,18 +903,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
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH 08/15] drm/i915: Add NV12 as supported format for primary plane
2015-08-20 1:02 ` [PATCH 08/15] drm/i915: Add NV12 as supported format for primary plane Chandra Konduru
@ 2015-08-26 8:40 ` Daniel Vetter
2015-08-27 1:40 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Daniel Vetter @ 2015-08-26 8:40 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:29PM -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
I think it's time to unify the separate primary/cursor code we have for
skl and just use the sprite one everywhere. Doesn't need to happen
necessarily before this patch series, but really needs to happen rather
sooner than later. We've already broken some of the scaler tests because
we had to hack away the 3rd plane because we couldn't remove the cursor
stuff due to some oddball (and undebugged) reason. Iirc Damien has some
patches somewhere for this.
-Daniel
> ---
> 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 b1d9edf..e4a6a91 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,
> @@ -13606,8 +13619,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
--
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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-08-20 1:02 ` [PATCH 14/15] drm/i915: skl nv12 workarounds Chandra Konduru
@ 2015-08-26 8:42 ` Daniel Vetter
2015-08-27 1:44 ` Konduru, Chandra
2015-09-04 11:26 ` Ville Syrjälä
1 sibling, 1 reply; 57+ messages in thread
From: Daniel Vetter @ 2015-08-26 8:42 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:35PM -0700, Chandra Konduru wrote:
> Adding driver workarounds for nv12.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
> drivers/gpu/drm/i915/intel_csr.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_sprite.c | 7 +++++++
> 5 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c4d732f..3192837 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5354,6 +5354,26 @@ 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)
> +
> +#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_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index ba1ae03..559a7f5 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -181,7 +181,7 @@ static const struct stepping_info skl_stepping_info[] = {
> {'G', '0'}, {'H', '0'}, {'I', '0'}
> };
>
> -static char intel_get_stepping(struct drm_device *dev)
> +char intel_get_stepping(struct drm_device *dev)
I guess we should have a new home for this now that it's used outside of
intel_csr.c Plus kerneldoc, as usual.
> {
> if (IS_SKYLAKE(dev) && (dev->pdev->revision <
> ARRAY_SIZE(skl_stepping_info)))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 419660d..2158b8f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3196,6 +3196,16 @@ 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);
>
> + DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> + IS_SKYLAKE(dev), IS_BROXTON(dev));
> +
Wa comments are missing here. Also generally we do 1 wa per commit.
-Daniel
> + 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;
>
> @@ -5004,6 +5014,21 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
> return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
> }
>
> +
> +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;
> @@ -5094,6 +5119,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)
> @@ -5211,6 +5239,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)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d50b8cb..63750d5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1158,6 +1158,7 @@ void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
> enum csr_state state);
> void intel_csr_load_program(struct drm_device *dev);
> void intel_csr_ucode_fini(struct drm_device *dev);
> +char intel_get_stepping(struct drm_device *dev);
> void assert_csr_loaded(struct drm_i915_private *dev_priv);
>
> /* intel_dp.c */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0ea9273..9d1c5b9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -278,6 +278,13 @@ 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);
>
> + 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
--
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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 08/15] drm/i915: Add NV12 as supported format for primary plane
2015-08-26 8:40 ` Daniel Vetter
@ 2015-08-27 1:40 ` Konduru, Chandra
0 siblings, 0 replies; 57+ messages in thread
From: Konduru, Chandra @ 2015-08-27 1:40 UTC (permalink / raw
To: Daniel Vetter
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, August 26, 2015 1:40 AM
> To: Konduru, Chandra
> Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville
> Subject: Re: [Intel-gfx] [PATCH 08/15] drm/i915: Add NV12 as supported format
> for primary plane
>
> On Wed, Aug 19, 2015 at 06:02:29PM -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
>
> I think it's time to unify the separate primary/cursor code we have for
> skl and just use the sprite one everywhere. Doesn't need to happen
> necessarily before this patch series, but really needs to happen rather
> sooner than later. We've already broken some of the scaler tests because
> we had to hack away the 3rd plane because we couldn't remove the cursor
> stuff due to some oddball (and undebugged) reason. Iirc Damien has some
> patches somewhere for this.
> -Daniel
>
I didn't planned before this patch series, but fully agree with
your assessment to unify.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-08-26 8:42 ` Daniel Vetter
@ 2015-08-27 1:44 ` Konduru, Chandra
2015-09-02 8:02 ` Daniel Vetter
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-08-27 1:44 UTC (permalink / raw
To: Daniel Vetter
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > -static char intel_get_stepping(struct drm_device *dev)
> > +char intel_get_stepping(struct drm_device *dev)
>
> I guess we should have a new home for this now that it's used outside of
> intel_csr.c Plus kerneldoc, as usual.
Will add kerneldoc header and respun, but where do you want to move to?
>
> > {
> > if (IS_SKYLAKE(dev) && (dev->pdev->revision <
> > ARRAY_SIZE(skl_stepping_info)))
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> > index 419660d..2158b8f 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3196,6 +3196,16 @@ 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);
> >
> > + DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> > + IS_SKYLAKE(dev), IS_BROXTON(dev));
> > +
>
> Wa comments are missing here. Also generally we do 1 wa per commit.
> -Daniel
Sure, will split and respun.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-08-27 1:44 ` Konduru, Chandra
@ 2015-09-02 8:02 ` Daniel Vetter
2015-09-03 18:33 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Daniel Vetter @ 2015-09-02 8:02 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
On Thu, Aug 27, 2015 at 01:44:06AM +0000, Konduru, Chandra wrote:
> > > -static char intel_get_stepping(struct drm_device *dev)
> > > +char intel_get_stepping(struct drm_device *dev)
> >
> > I guess we should have a new home for this now that it's used outside of
> > intel_csr.c Plus kerneldoc, as usual.
>
> Will add kerneldoc header and respun, but where do you want to move to?
If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
better home.
-Daniel
>
> >
> > > {
> > > if (IS_SKYLAKE(dev) && (dev->pdev->revision <
> > > ARRAY_SIZE(skl_stepping_info)))
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > > index 419660d..2158b8f 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -3196,6 +3196,16 @@ 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);
> > >
> > > + DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> > > + IS_SKYLAKE(dev), IS_BROXTON(dev));
> > > +
> >
> > Wa comments are missing here. Also generally we do 1 wa per commit.
> > -Daniel
>
> Sure, will split and respun.
>
--
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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-02 8:02 ` Daniel Vetter
@ 2015-09-03 18:33 ` Konduru, Chandra
2015-09-04 7:40 ` Daniel Vetter
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-03 18:33 UTC (permalink / raw
To: Daniel Vetter
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, September 02, 2015 1:02 AM
> To: Konduru, Chandra
> Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville
> Subject: Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds
>
> On Thu, Aug 27, 2015 at 01:44:06AM +0000, Konduru, Chandra wrote:
> > > > -static char intel_get_stepping(struct drm_device *dev)
> > > > +char intel_get_stepping(struct drm_device *dev)
> > >
> > > I guess we should have a new home for this now that it's used outside of
> > > intel_csr.c Plus kerneldoc, as usual.
> >
> > Will add kerneldoc header and respun, but where do you want to move to?
>
> If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
> better home.
> -Daniel
I sent updated patch series couple days ago moving to intel_display.c.
I hope that is fine.
By the way, can you start giving RB tags for the patches you reviewed?
>
> >
> > >
> > > > {
> > > > if (IS_SKYLAKE(dev) && (dev->pdev->revision <
> > > > ARRAY_SIZE(skl_stepping_info)))
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index 419660d..2158b8f 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -3196,6 +3196,16 @@ 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);
> > > >
> > > > + DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> > > > + IS_SKYLAKE(dev), IS_BROXTON(dev));
> > > > +
> > >
> > > Wa comments are missing here. Also generally we do 1 wa per commit.
> > > -Daniel
> >
> > Sure, will split and respun.
> >
>
> --
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-03 18:33 ` Konduru, Chandra
@ 2015-09-04 7:40 ` Daniel Vetter
2015-09-05 2:09 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Daniel Vetter @ 2015-09-04 7:40 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
On Thu, Sep 03, 2015 at 06:33:25PM +0000, Konduru, Chandra wrote:
>
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> > Sent: Wednesday, September 02, 2015 1:02 AM
> > To: Konduru, Chandra
> > Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Vetter, Daniel; Syrjala, Ville
> > Subject: Re: [Intel-gfx] [PATCH 14/15] drm/i915: skl nv12 workarounds
> >
> > On Thu, Aug 27, 2015 at 01:44:06AM +0000, Konduru, Chandra wrote:
> > > > > -static char intel_get_stepping(struct drm_device *dev)
> > > > > +char intel_get_stepping(struct drm_device *dev)
> > > >
> > > > I guess we should have a new home for this now that it's used outside of
> > > > intel_csr.c Plus kerneldoc, as usual.
> > >
> > > Will add kerneldoc header and respun, but where do you want to move to?
> >
> > If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
> > better home.
> > -Daniel
> I sent updated patch series couple days ago moving to intel_display.c.
> I hope that is fine.
Imo intel_display.c is the wrong place since this isn't display-specific
really - stepping is for the entire device. intel_uncore.c, i915_drv.c or
i915_dma.c would all be suitable places.
> By the way, can you start giving RB tags for the patches you reviewed?
This isn't a full review, just a small thing I spotted while reading
patches.
-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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec
2015-08-20 1:02 ` [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec Chandra Konduru
@ 2015-09-04 8:17 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 8:17 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:22PM -0700, Chandra Konduru wrote:
> Properly allocate min blocks per hw requirements.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 39 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fff0c22..da3046f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2959,6 +2959,41 @@ 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, int y_plane)
bool y_plane
In general I dislike the fact that the code treats Y as the special case
instead of CbCr. The oppostire would be far more natural IMO, and,
I believe, would result in less checks in the code all around. Althouh
I think in general we should just pass around the format and plane index.
> +{
> + uint16_t min_alloc;
> +
> + /* For packed formats, no y-plane, return 0 */
> + if (y_plane && !p->y_bytes_per_pixel)
> + return 0;
> +
> +
Extra line.
> + 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;
> +
> + switch (bytes_per_pixel) {
> + case 1:
> + min_scanlines = 32;
> + break;
> + case 2:
> + min_scanlines = 16;
> + break;
> + case 8:
> + WARN(1, "Unsupported pixel depth for rotation");
> + }
Could be just 32/cpp.
> + min_alloc = DIV_ROUND_UP((4 * p->horiz_pixels/(y_plane ? 1 : 2) *
> + bytes_per_pixel), 512) * min_scanlines/4 + 3;
Another case that could be simplified by removing the y_plane special
casing. In fact just passing in the format and plane index in we could get:
cpp = drm_format_plane_cpp(format, plane);
width = width / drm_format_horiz_subsampling(format);
min_scanlines = 32 / cpp;
min_alloc = DIV_ROUND_UP(4 * width * cpp, 512) * min_scanlines / 4 + 3;
We could even move the width/subsampling (and height too) thing into a
common helper in a drm header, eg.:
static inline int drm_format_plane_width(format, plane, width)
{
if (plane)
return width / drm_format_plane_horiz_subsampling(format);
else
return width;
}
static inline int drm_format_plane_height(format, plane, height);
{
if (plane)
return height / drm_format_plane_vert_subsampling(format);
else
return height;
}
> + } 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 +3034,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> if (!p->enabled)
> continue;
>
> - minimum[plane] = 8;
> + minimum[plane] = skl_dbuf_min_alloc(p, 0); /* 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, 1); /* y-plane */
false/true instead of 0/1.
With the bool things changed this does what it says so:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h
2015-08-20 1:02 ` [PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h Chandra Konduru
@ 2015-09-04 8:31 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 8:31 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:23PM -0700, Chandra Konduru wrote:
> This patch swaps src width and height for dbuf/wm calculations
> when rotation is 90/270 as per hw requirements.
The spec is rather unclear about this. It only says:
"If plane 90 or 270 rotation is enabled, use the rotated width and
height in pixel rate calculations."
Although it does make sense based on the fact that the display
fetch will happen in the rotated direction.
I tried to reconcile some if this stuff in my head but when I went to
read the Yf/Ys tiling documentation I got even more confused. What it
says there is that a single cacheline always contains 4 rows of pixels,
but then in the next sentence it claims the aspect ratio will be 1:1
for 8,32,128 bpp, and 2:1 for 16,64 bpp. Those two things can't both be
true. Can someone tell me which one is true?
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++++++++++----
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index da3046f..c455946 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3193,6 +3193,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> enum pipe pipe = intel_crtc->pipe;
> struct drm_plane *plane;
> struct drm_framebuffer *fb;
> + struct intel_plane_state *plane_state;
> + int src_w, src_h;
These can be moved to a narrower scope. plane_state could be const I
believe.
> int i = 1; /* Index for sprite planes start */
>
> p->active = intel_crtc->active;
> @@ -3201,6 +3203,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> 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;
> @@ -3215,8 +3218,22 @@ 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;
> +
> + if (drm_rect_width(&plane_state->src)) {
> + src_w = drm_rect_width(&plane_state->src) >> 16;
> + src_h = drm_rect_height(&plane_state->src) >> 16;
> + } else {
> + src_w = intel_crtc->config->pipe_src_w;
> + src_h = intel_crtc->config->pipe_src_h;
> + }
Are we still not having a consistent plane state all the time, or why is
this kludge needed?
> +
> + 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;
> @@ -3750,8 +3767,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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 03/15] drm/i915: Add register definitions for NV12 support
2015-08-20 1:02 ` [PATCH 03/15] drm/i915: Add register definitions for NV12 support Chandra Konduru
@ 2015-09-04 8:40 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 8:40 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:24PM -0700, Chandra Konduru wrote:
> This patch adds register definitions for skylake
> display NV12 support.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1fa0554..c4d732f 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)
> @@ -5601,6 +5602,32 @@ 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))
> +
Bspec seems entirely confused about these registers, not showing
AUX_OFFSET for SKL at all, and even the normal OFFSET has lost all the
information about the register offsets. But it does match what's there
for gen10, so I suppose it's all good.
But this stuff could be squashed to whichever patch(es) actually use the
new defines.
> /* legacy palette */
> #define _LGC_PALETTE_A 0x4a000
> #define _LGC_PALETTE_B 0x4a800
> --
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 04/15] drm/i915: Set scaler mode for NV12
2015-08-20 1:02 ` [PATCH 04/15] drm/i915: Set scaler mode for NV12 Chandra Konduru
@ 2015-09-04 8:53 ` Ville Syrjälä
2015-09-04 15:03 ` Daniel Vetter
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 8:53 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:25PM -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.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> 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.
I was almost going to say that we don't have code like this, but then I
found it hiding in intel_atomic.c for whatever reason.
Anyway, the patch looks good so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
On a further note, this function could use some cleaning to move
various variables into narrower scope. Now it's rather hard to see what
is valid per iteration and what is valid across the entire loop.
> --
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 05/15] drm/i915: Stage scaler request for NV12 as src format
2015-08-20 1:02 ` [PATCH 05/15] drm/i915: Stage scaler request for NV12 as src format Chandra Konduru
@ 2015-09-04 10:17 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 10:17 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:26PM -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)
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++++++++++------
> 1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3ee1c17..411b211 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4341,10 +4341,27 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
> }
> }
>
> +static int skl_need_scaling(int src_w, int dst_w, int src_h, int dst_h,
Make it bool.
I'd reorder the arguments to group src_w and src_h together,
and dst_w and dst_h together.
> + int rotation, uint32_t pixel_format)
Indentation is weird. rotation should be unsigned int.
> +{
> + /* scaling is required when src dst sizes doesn't match or format is NV12 */
> + if (src_w != dst_w || src_h != dst_h)
> + return 1;
> +
> + if (intel_rotation_90_or_270(rotation) &&
> + (src_h != dst_w || src_w != dst_h))
> + return 1;
I would make this something like:
{
if (format == NV12)
return true;
if (90_270)
return ...;
else
return ...;
}
> +
> + if (pixel_format == DRM_FORMAT_NV12)
> + return 1;
> +
> + return 0;
> +}
> +
> 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;
> @@ -4352,9 +4369,8 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
> to_intel_crtc(crtc_state->base.crtc);
> int 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, dst_w, src_h, dst_h, rotation,
> + pixel_format);
>
> /*
> * if plane is being disabled or scaler is no more required or force detach
> @@ -4423,7 +4439,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 +4475,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 +4501,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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 06/15] drm/i915: Update format_is_yuv() to include NV12
2015-08-20 1:02 ` [PATCH 06/15] drm/i915: Update format_is_yuv() to include NV12 Chandra Konduru
@ 2015-09-04 10:17 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 10:17 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:27PM -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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 07/15] drm/i915: Upscale scaler max scale for NV12.
2015-08-20 1:02 ` [PATCH 07/15] drm/i915: Upscale scaler max scale for NV12 Chandra Konduru
@ 2015-09-04 10:22 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 10:22 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:28PM -0700, Chandra Konduru wrote:
> This patch updates max supported scaler limits for NV12.
>
> v2:
> -Rebased to current kernel version 4.2.0.rc4 (me)
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 13 +++++++++----
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 411b211..b1d9edf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13421,7 +13421,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;
> @@ -13441,11 +13443,13 @@ 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));
> + max_scale = min((1 << 16) * (pixel_format == DRM_FORMAT_NV12 ? 2 : 3) - 1,
> + (1 << 8) * ((cdclk << 8) / crtc_clock));
Starting to be a bit messy.
Maybe something like
if (nv12)
max_scale = (2 << 16) - 1;
else
max_scale = (3 << 16) - 1;
max_scale = min(max_scale, ...);
?
>
> return max_scale;
> }
> @@ -13465,7 +13469,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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 09/15] drm/i915: Add NV12 as supported format for sprite plane
2015-08-20 1:02 ` [PATCH 09/15] drm/i915: Add NV12 as supported format for sprite plane Chandra Konduru
@ 2015-09-04 10:28 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 10:28 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:30PM -0700, Chandra Konduru wrote:
> This patch adds NV12 to list of supported formats for
> sprite plane.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12
> ---
> drivers/gpu/drm/i915/intel_sprite.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 66d60ae..4e8c020 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1041,6 +1041,19 @@ static uint32_t skl_plane_formats[] = {
> DRM_FORMAT_VYUY,
> };
>
> +static uint32_t skl_plane_formats_with_nv12[] = {
static const ...
> + 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) - 1;
Stray '-1'
> + }
Only the first two planes have NV12. So primary and sprite 0 for us.
And none on pipe C. Yep, matches the spec.
With the const and -1 fixed this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> +
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init
2015-08-20 1:02 ` [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init Chandra Konduru
@ 2015-09-04 10:40 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 10:40 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:31PM -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)
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12
> ---
> drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e4a6a91..4df4d77 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14343,6 +14343,34 @@ 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;
> + }
I'm not sure this check makes much sense. We should perhaps either reject
all cases where the planes overlap, or allow it all.
Some years ago I was thinking of adding overlap check to drm core, but
then I figured maybe someone wants to interleave the planes in memory,
so the overlap checks would then have to check each line for overlap
so it gets a bit nasty. So I'm not sure there's any point in disallowing
overlapping planes.
> + if (mode_cmd->pitches[0] != mode_cmd->pitches[1] ||
> + mode_cmd->handles[0] != mode_cmd->handles[1]) {
> + DRM_DEBUG("y and uv subplanes have different parameters\n");
> + return -EINVAL;
> + }
Maybe we'd want a separate debug message for these two. I think the
shared stride limitation would be fairly common in other hardware, but
the fact that we require the bo to be the same is unfortunately quite
a special "feature" of our latest hardware. So having a clear debug
message for it might help people figure out why their seemingly valid
code doesn't work on i915.
> + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
> + (mode_cmd->offsets[1] & 0xFFF)) {
Bunch of indentation problems in this patch as well.
> + 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));
> --
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-08-20 1:02 ` [PATCH 11/15] drm/i915: Add NV12 to primary plane programming Chandra Konduru
@ 2015-09-04 11:09 ` Ville Syrjälä
2015-09-04 15:06 ` Daniel Vetter
2015-09-05 1:10 ` Konduru, Chandra
0 siblings, 2 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 11:09 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:32PM -0700, Chandra Konduru wrote:
> 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)
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Testcase: igt/kms_nv12
> ---
> drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4df4d77..329651e 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 ?
> + stride / 2 : stride;
The 2x part was rather well hidden in the spec. How do we deal with
cases when the Y stride is an odd number of tiles?
> + }
> }
> 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);
> }
>
> @@ -11626,6 +11657,12 @@ 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;
>
> + /* Adjust (macro)pixel boundary */
> + if (fb && intel_format_is_yuv(fb->pixel_format)) {
> + to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
> + to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
> + }
That can increase the size of the source rectangle, so it needs to be done
in a way that avois that. And we already do this for sprites.
Really, someone should just finally fix the mess we have made with handling
different types of planes in totally different ways and unify things as
much as possible.
> +
> if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
> plane->type != DRM_PLANE_TYPE_CURSOR) {
> ret = skl_update_scaler_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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 13/15] drm/i915: Set initial phase & trip for NV12 scaler
2015-08-20 1:02 ` [PATCH 13/15] drm/i915: Set initial phase & trip for NV12 scaler Chandra Konduru
@ 2015-09-04 11:15 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 11:15 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:34PM -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.
We should set it according to the sub-pixel coordinates. At the moment
we just throw away the sub-pixel parts, but we should change that and do
it right(tm).
And yes some chroma siting properties would also be nice in the future.
I proposed something for that long ago, but that predates even plane
properties IIRC, so it was some ioctl based junk.
>
> 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 329651e..419660d 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 ?
> 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 a1384a7..0ea9273 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 ?
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-08-20 1:02 ` [PATCH 14/15] drm/i915: skl nv12 workarounds Chandra Konduru
2015-08-26 8:42 ` Daniel Vetter
@ 2015-09-04 11:26 ` Ville Syrjälä
2015-09-05 1:28 ` Konduru, Chandra
1 sibling, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 11:26 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:35PM -0700, Chandra Konduru wrote:
> Adding driver workarounds for nv12.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 20 ++++++++++++++++++++
> drivers/gpu/drm/i915/intel_csr.c | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 31 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_sprite.c | 7 +++++++
> 5 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c4d732f..3192837 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5354,6 +5354,26 @@ 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)
> +
> +#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_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index ba1ae03..559a7f5 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -181,7 +181,7 @@ static const struct stepping_info skl_stepping_info[] = {
> {'G', '0'}, {'H', '0'}, {'I', '0'}
> };
>
> -static char intel_get_stepping(struct drm_device *dev)
> +char intel_get_stepping(struct drm_device *dev)
> {
> if (IS_SKYLAKE(dev) && (dev->pdev->revision <
> ARRAY_SIZE(skl_stepping_info)))
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 419660d..2158b8f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3196,6 +3196,16 @@ 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);
>
> + DRM_DEBUG_KMS("KCM: is_skl = %d is_bxt = %d\n",
> + IS_SKYLAKE(dev), IS_BROXTON(dev));
> +
> + 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);
> + }
According to Bspec this would need to be disabled for render
compression. And to do that we'd need to add some vblank waits to make
sure we don't disable it too soon. But since it's pre-production
hardware anyway I guess we might not care too much.
I would probably drop SKL from these since I'd assume almost everyone
has D+ by now. And maybe just stuff it in init_clock_gating for BXT
since we're going to eliminate it soonish anyway.
> +
> if (scaler_id >= 0) {
> uint32_t ps_ctrl = 0;
>
> @@ -5004,6 +5014,21 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc)
> return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A;
> }
>
> +
> +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;
> @@ -5094,6 +5119,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);
I wonder what's the cost of having this
a) always enabled
b) enabled when the pipe is enabled
c) enabled only when NV12 is used
?
> }
>
> static void ironlake_pfit_disable(struct intel_crtc *crtc)
> @@ -5211,6 +5239,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)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d50b8cb..63750d5 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1158,6 +1158,7 @@ void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
> enum csr_state state);
> void intel_csr_load_program(struct drm_device *dev);
> void intel_csr_ucode_fini(struct drm_device *dev);
> +char intel_get_stepping(struct drm_device *dev);
> void assert_csr_loaded(struct drm_i915_private *dev_priv);
>
> /* intel_dp.c */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0ea9273..9d1c5b9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -278,6 +278,13 @@ 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);
>
> + 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format.
2015-08-20 1:02 ` [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format Chandra Konduru
@ 2015-09-04 11:30 ` Ville Syrjälä
2015-09-05 1:38 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-04 11:30 UTC (permalink / raw
To: Chandra Konduru; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Wed, Aug 19, 2015 at 06:02:36PM -0700, Chandra Konduru wrote:
> 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 | 28 +++++++++++------
> drivers/gpu/drm/i915/intel_sprite.c | 56 ++++++++++++++++++++++------------
> 2 files changed, 56 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2158b8f..19d0f8b 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;
> @@ -11697,8 +11702,13 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>
> /* Adjust (macro)pixel boundary */
> if (fb && intel_format_is_yuv(fb->pixel_format)) {
> - to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
> - to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
> + if (intel_rotation_90_or_270(plane_state->rotation)) {
> + to_intel_plane_state(plane_state)->src.y1 &= ~0x10000;
> + to_intel_plane_state(plane_state)->src.y2 &= ~0x10000;
> + } else {
> + to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
> + to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
> + }
IIRC we concluded (with Art's help) that this is not needed. We always
want to align in src.x.
> }
>
> 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 9d1c5b9..3522cb0 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;
> @@ -900,18 +903,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;
> + }
Same here.
> }
> }
>
> --
> 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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 04/15] drm/i915: Set scaler mode for NV12
2015-09-04 8:53 ` Ville Syrjälä
@ 2015-09-04 15:03 ` Daniel Vetter
0 siblings, 0 replies; 57+ messages in thread
From: Daniel Vetter @ 2015-09-04 15:03 UTC (permalink / raw
To: Ville Syrjälä; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Fri, Sep 04, 2015 at 11:53:36AM +0300, Ville Syrjälä wrote:
> On a further note, this function could use some cleaning to move
> various variables into narrower scope. Now it's rather hard to see what
> is valid per iteration and what is valid across the entire loop.
It's also rather big, so might just want to split out the inner parts
perhaps into helper functions.
-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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-04 11:09 ` Ville Syrjälä
@ 2015-09-04 15:06 ` Daniel Vetter
2015-09-05 1:10 ` Konduru, Chandra
1 sibling, 0 replies; 57+ messages in thread
From: Daniel Vetter @ 2015-09-04 15:06 UTC (permalink / raw
To: Ville Syrjälä; +Cc: daniel.vetter, intel-gfx, ville.syrjala
On Fri, Sep 04, 2015 at 02:09:41PM +0300, Ville Syrjälä wrote:
> Really, someone should just finally fix the mess we have made with handling
> different types of planes in totally different ways and unify things as
> much as possible.
Yeah I want universal planes for skl finally, and I'd like to have someon
on the hook and commited to do it before I pull in this series. We pushed
this task off for almost a year by now, and it's been endlessly pain.
-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
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-04 11:09 ` Ville Syrjälä
2015-09-04 15:06 ` Daniel Vetter
@ 2015-09-05 1:10 ` Konduru, Chandra
2015-09-05 14:59 ` Ville Syrjälä
1 sibling, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-05 1:10 UTC (permalink / raw
To: Ville Syrjälä
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > +
> > + 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 ?
> > + stride / 2 : stride;
>
> The 2x part was rather well hidden in the spec. How do we deal with
> cases when the Y stride is an odd number of tiles?
It should be a round up division to take care of that scenario.
Fixing this and resolving your other comments.
Will be sending updated patches shortly.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-04 11:26 ` Ville Syrjälä
@ 2015-09-05 1:28 ` Konduru, Chandra
2015-09-05 14:52 ` Ville Syrjälä
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-05 1:28 UTC (permalink / raw
To: Ville Syrjälä
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > +
> > + 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);
> > + }
>
> According to Bspec this would need to be disabled for render
> compression. And to do that we'd need to add some vblank waits to make
> sure we don't disable it too soon. But since it's pre-production
> hardware anyway I guess we might not care too much.
Render compression related checks will be coming as part of that patch.
>
> I would probably drop SKL from these since I'd assume almost everyone
> has D+ by now. And maybe just stuff it in init_clock_gating for BXT
> since we're going to eliminate it soonish anyway.
Last checked some folks are still using, so keep it intact for completeness.
> > +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;
> > @@ -5094,6 +5119,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);
>
> I wonder what's the cost of having this
> a) always enabled
> b) enabled when the pipe is enabled
> c) enabled only when NV12 is used
> ?
Initially optimized to enable only when nv12 is used,
but there are some corner cases when planes switch to and
from nv12 to non-nv12 and SV recommendation is to enable
always; and SV evaluated cost, and it isn't a big concern.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format.
2015-09-04 11:30 ` Ville Syrjälä
@ 2015-09-05 1:38 ` Konduru, Chandra
2015-09-05 14:48 ` Ville Syrjälä
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-05 1:38 UTC (permalink / raw
To: Ville Syrjälä, Runyan, Arthur J
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > /* Adjust (macro)pixel boundary */
> > if (fb && intel_format_is_yuv(fb->pixel_format)) {
> > - to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
> > - to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
> > + if (intel_rotation_90_or_270(plane_state->rotation)) {
> > + to_intel_plane_state(plane_state)->src.y1 &=
> ~0x10000;
> > + to_intel_plane_state(plane_state)->src.y2 &=
> ~0x10000;
> > + } else {
> > + to_intel_plane_state(plane_state)->src.x1 &=
> ~0x10000;
> > + to_intel_plane_state(plane_state)->src.x2 &=
> ~0x10000;
> > + }
>
> IIRC we concluded (with Art's help) that this is not needed. We always
> want to align in src.x.
Initial code that I added was making both X and Y offsets as even
when 90/270 as per bspec at that time.
But later Art update is as below:
>> " the X offset must always be even for YUV422+NV12, and
>> the Y offset must be even when rotated 90/270 degrees."
So, above code change is needed.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-04 7:40 ` Daniel Vetter
@ 2015-09-05 2:09 ` Konduru, Chandra
0 siblings, 0 replies; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-05 2:09 UTC (permalink / raw
To: Daniel Vetter
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > > On Thu, Aug 27, 2015 at 01:44:06AM +0000, Konduru, Chandra wrote:
> > > > > > -static char intel_get_stepping(struct drm_device *dev)
> > > > > > +char intel_get_stepping(struct drm_device *dev)
> > > > >
> > > > > I guess we should have a new home for this now that it's used outside of
> > > > > intel_csr.c Plus kerneldoc, as usual.
> > > >
> > > > Will add kerneldoc header and respun, but where do you want to move to?
> > >
> > > If you want my dice-roll, I'd shovel it into intel_uncore.c for lack of
> > > better home.
> > > -Daniel
> > I sent updated patch series couple days ago moving to intel_display.c.
> > I hope that is fine.
>
> Imo intel_display.c is the wrong place since this isn't display-specific
> really - stepping is for the entire device. intel_uncore.c, i915_drv.c or
> i915_dma.c would all be suitable places.
Moved to i915_drv.c. Sending out updated patches shortly.
>
> > By the way, can you start giving RB tags for the patches you reviewed?
>
> This isn't a full review, just a small thing I spotted while reading
> patches.
OK, Ville did/is doing full review. So covered.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format.
2015-09-05 1:38 ` Konduru, Chandra
@ 2015-09-05 14:48 ` Ville Syrjälä
0 siblings, 0 replies; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-05 14:48 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Runyan, Arthur J,
Syrjala, Ville
On Sat, Sep 05, 2015 at 01:38:14AM +0000, Konduru, Chandra wrote:
> > > /* Adjust (macro)pixel boundary */
> > > if (fb && intel_format_is_yuv(fb->pixel_format)) {
> > > - to_intel_plane_state(plane_state)->src.x1 &= ~0x10000;
> > > - to_intel_plane_state(plane_state)->src.x2 &= ~0x10000;
> > > + if (intel_rotation_90_or_270(plane_state->rotation)) {
> > > + to_intel_plane_state(plane_state)->src.y1 &=
> > ~0x10000;
> > > + to_intel_plane_state(plane_state)->src.y2 &=
> > ~0x10000;
> > > + } else {
> > > + to_intel_plane_state(plane_state)->src.x1 &=
> > ~0x10000;
> > > + to_intel_plane_state(plane_state)->src.x2 &=
> > ~0x10000;
> > > + }
> >
> > IIRC we concluded (with Art's help) that this is not needed. We always
> > want to align in src.x.
>
> Initial code that I added was making both X and Y offsets as even
> when 90/270 as per bspec at that time.
>
> But later Art update is as below:
> >> " the X offset must always be even for YUV422+NV12, and
> >> the Y offset must be even when rotated 90/270 degrees."
>
> So, above code change is needed.
That's the orignal text, later update from Art was:
"They don't both have to be even when roated. The text should say " the
X offset must be even for YUV422+NV12 ***when not rotated 90/270***, and
the Y offset must be even when rotated 90/270 degrees.""
And the spec has been updated since to include the same information. For
us we just need to align src.x? since that's what gets programmed into
the X offset for 0/180, and into the Y offset for 90/270. So the current
code is fine as is.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-05 1:28 ` Konduru, Chandra
@ 2015-09-05 14:52 ` Ville Syrjälä
2015-09-08 23:51 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-05 14:52 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
On Sat, Sep 05, 2015 at 01:28:56AM +0000, Konduru, Chandra wrote:
> > > +
> > > + 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);
> > > + }
> >
> > According to Bspec this would need to be disabled for render
> > compression. And to do that we'd need to add some vblank waits to make
> > sure we don't disable it too soon. But since it's pre-production
> > hardware anyway I guess we might not care too much.
>
> Render compression related checks will be coming as part of that patch.
>
> >
> > I would probably drop SKL from these since I'd assume almost everyone
> > has D+ by now. And maybe just stuff it in init_clock_gating for BXT
> > since we're going to eliminate it soonish anyway.
>
> Last checked some folks are still using, so keep it intact for completeness.
>
> > > +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;
> > > @@ -5094,6 +5119,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);
> >
> > I wonder what's the cost of having this
> > a) always enabled
> > b) enabled when the pipe is enabled
> > c) enabled only when NV12 is used
> > ?
>
> Initially optimized to enable only when nv12 is used,
> but there are some corner cases when planes switch to and
> from nv12 to non-nv12 and SV recommendation is to enable
> always; and SV evaluated cost, and it isn't a big concern.
So, based on that we could just stuff it into init_clock_gating and
forget about it.
But we'll run into problems as soon as render compression enters the
picure. But I don't have a problem leaving it up to the render
compression patches to solve.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-05 1:10 ` Konduru, Chandra
@ 2015-09-05 14:59 ` Ville Syrjälä
2015-09-08 23:30 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-05 14:59 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
On Sat, Sep 05, 2015 at 01:10:11AM +0000, Konduru, Chandra wrote:
> > > +
> > > + 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 ?
> > > + stride / 2 : stride;
> >
> > The 2x part was rather well hidden in the spec. How do we deal with
> > cases when the Y stride is an odd number of tiles?
>
> It should be a round up division to take care of that scenario.
That would stil lresult in a corrupted picture I think. So I was
thinking that we should just refuse to create NCV12 framebuffers with a
poorly aligned stride.
> Fixing this and resolving your other comments.
> Will be sending updated patches shortly.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-05 14:59 ` Ville Syrjälä
@ 2015-09-08 23:30 ` Konduru, Chandra
2015-09-09 11:41 ` Ville Syrjälä
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-08 23:30 UTC (permalink / raw
To: Ville Syrjälä
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > > > + 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 ?
> > > > + stride / 2 : stride;
> > >
> > > The 2x part was rather well hidden in the spec. How do we deal with
> > > cases when the Y stride is an odd number of tiles?
> >
> > It should be a round up division to take care of that scenario.
>
> That would stil lresult in a corrupted picture I think. So I was
> thinking that we should just refuse to create NCV12 framebuffers with a
> poorly aligned stride.
>
I added a check in intel_framebuffer_init() which should catch them:
if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
DRM_DEBUG("y and uv subplanes have different pitches\n");
return -EINVAL;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-05 14:52 ` Ville Syrjälä
@ 2015-09-08 23:51 ` Konduru, Chandra
2015-09-09 11:46 ` Ville Syrjälä
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-08 23:51 UTC (permalink / raw
To: Ville Syrjälä
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > > > +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;
> > > > @@ -5094,6 +5119,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);
> > >
> > > I wonder what's the cost of having this
> > > a) always enabled
> > > b) enabled when the pipe is enabled
> > > c) enabled only when NV12 is used
> > > ?
> >
> > Initially optimized to enable only when nv12 is used,
> > but there are some corner cases when planes switch to and
> > from nv12 to non-nv12 and SV recommendation is to enable
> > always; and SV evaluated cost, and it isn't a big concern.
>
> So, based on that we could just stuff it into init_clock_gating and
> forget about it.
Couldn't include into init_clock_gating because this requires
a pipe based check.
By the way, so far 4 patches got RB tags.
In the respun series (http://lists.freedesktop.org/archives/intel-gfx/2015-September/075235.html
addressed your feedback), those 4 tags goes to 1, 3, 5 and 8 of 15.
Can you check updated patches and issue R-B tags for remaining ones?
>
> But we'll run into problems as soon as render compression enters the
> picure. But I don't have a problem leaving it up to the render
> compression patches to solve.
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-08 23:30 ` Konduru, Chandra
@ 2015-09-09 11:41 ` Ville Syrjälä
2015-09-09 17:12 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-09 11:41 UTC (permalink / raw
To: Konduru, Chandra; +Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org
On Wed, Sep 09, 2015 at 02:30:58AM +0300, Konduru, Chandra wrote:
> > > > > + 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 ?
> > > > > + stride / 2 : stride;
> > > >
> > > > The 2x part was rather well hidden in the spec. How do we deal with
> > > > cases when the Y stride is an odd number of tiles?
> > >
> > > It should be a round up division to take care of that scenario.
> >
> > That would stil lresult in a corrupted picture I think. So I was
> > thinking that we should just refuse to create NCV12 framebuffers with a
> > poorly aligned stride.
> >
> I added a check in intel_framebuffer_init() which should catch them:
> if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> DRM_DEBUG("y and uv subplanes have different pitches\n");
> return -EINVAL;
> }
That won't catch the case I'm worried about. We would also need to make
sure pitches[1] is aligned to the UV tile width.
--
Ville Syrjälä
Intel OTC
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-08 23:51 ` Konduru, Chandra
@ 2015-09-09 11:46 ` Ville Syrjälä
2015-09-09 17:20 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-09 11:46 UTC (permalink / raw
To: Konduru, Chandra; +Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org
On Wed, Sep 09, 2015 at 02:51:58AM +0300, Konduru, Chandra wrote:
> > > > > +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;
> > > > > @@ -5094,6 +5119,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);
> > > >
> > > > I wonder what's the cost of having this
> > > > a) always enabled
> > > > b) enabled when the pipe is enabled
> > > > c) enabled only when NV12 is used
> > > > ?
> > >
> > > Initially optimized to enable only when nv12 is used,
> > > but there are some corner cases when planes switch to and
> > > from nv12 to non-nv12 and SV recommendation is to enable
> > > always; and SV evaluated cost, and it isn't a big concern.
> >
> > So, based on that we could just stuff it into init_clock_gating and
> > forget about it.
>
> Couldn't include into init_clock_gating because this requires
> a pipe based check.
init_clock_gating()
{
...
enable for pipe A
enable for pipe B
...
}
or
for_each_pipe(pipe)
if (pipe != C)
enable w/a
>
> By the way, so far 4 patches got RB tags.
> In the respun series (http://lists.freedesktop.org/archives/intel-gfx/2015-September/075235.html
> addressed your feedback), those 4 tags goes to 1, 3, 5 and 8 of 15.
> Can you check updated patches and issue R-B tags for remaining ones?
I think your trigger finger is a bit overly sensitive :) We still had
open issues in this series so posting another one makes things somewhat
messy.
--
Ville Syrjälä
Intel OTC
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-09 11:41 ` Ville Syrjälä
@ 2015-09-09 17:12 ` Konduru, Chandra
2015-09-09 18:05 ` Ville Syrjälä
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-09 17:12 UTC (permalink / raw
To: Syrjala, Ville; +Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org
> > > > > > + /* For tile-Yf, uv-subplane tile width is 2x of Y-
> subplane
> > > > > */
> > > > > > + aux_stride = fb->modifier[0] ==
> > > > > I915_FORMAT_MOD_Yf_TILED ?
> > > > > > + stride / 2 : stride;
> > > > >
> > > > > The 2x part was rather well hidden in the spec. How do we deal with
> > > > > cases when the Y stride is an odd number of tiles?
> > > >
> > > > It should be a round up division to take care of that scenario.
> > >
> > > That would stil lresult in a corrupted picture I think. So I was
> > > thinking that we should just refuse to create NCV12 framebuffers with a
> > > poorly aligned stride.
> > >
> > I added a check in intel_framebuffer_init() which should catch them:
> > if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> > DRM_DEBUG("y and uv subplanes have different pitches\n");
> > return -EINVAL;
> > }
>
> That won't catch the case I'm worried about. We would also need to make
> sure pitches[1] is aligned to the UV tile width.
If caller is following tile/row/pitch alignments properly for sub-planes of
NV12 Yf buffer, above check will catch.
But are you referring a case where userland isn't following tile/row size
alignments properly? In that case, above may not catch. But
isn't that is the case even with other FB formats if user land not
following tile/row/pitch alignments?
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 14/15] drm/i915: skl nv12 workarounds
2015-09-09 11:46 ` Ville Syrjälä
@ 2015-09-09 17:20 ` Konduru, Chandra
0 siblings, 0 replies; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-09 17:20 UTC (permalink / raw
To: Syrjala, Ville; +Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org
> > > > > > +
> > > > > > + /* workaround for NV12 */
> > > > > > + skl_wa_clkgate(dev_priv, pipe, 1);
> > > > >
> > > > > I wonder what's the cost of having this
> > > > > a) always enabled
> > > > > b) enabled when the pipe is enabled
> > > > > c) enabled only when NV12 is used
> > > > > ?
> > > >
> > > > Initially optimized to enable only when nv12 is used,
> > > > but there are some corner cases when planes switch to and
> > > > from nv12 to non-nv12 and SV recommendation is to enable
> > > > always; and SV evaluated cost, and it isn't a big concern.
> > >
> > > So, based on that we could just stuff it into init_clock_gating and
> > > forget about it.
> >
> > Couldn't include into init_clock_gating because this requires
> > a pipe based check.
>
> init_clock_gating()
> {
> ...
> enable for pipe A
> enable for pipe B
> ...
> }
>
> or
>
> for_each_pipe(pipe)
> if (pipe != C)
> enable w/a
>
Oh yeah, it can be done that way.
OK, will move it there.
>
> >
> > By the way, so far 4 patches got RB tags.
> > In the respun series (http://lists.freedesktop.org/archives/intel-gfx/2015-
> September/075235.html
> > addressed your feedback), those 4 tags goes to 1, 3, 5 and 8 of 15.
> > Can you check updated patches and issue R-B tags for remaining ones?
>
> I think your trigger finger is a bit overly sensitive :) We still had
> open issues in this series so posting another one makes things somewhat
> messy.
There is one other open relate to tile-Yf Y, UV plane pitch/stride check.
I just responded to that one.
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-09 17:12 ` Konduru, Chandra
@ 2015-09-09 18:05 ` Ville Syrjälä
2015-09-09 20:10 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-09 18:05 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
On Wed, Sep 09, 2015 at 05:12:06PM +0000, Konduru, Chandra wrote:
> > > > > > > + /* For tile-Yf, uv-subplane tile width is 2x of Y-
> > subplane
> > > > > > */
> > > > > > > + aux_stride = fb->modifier[0] ==
> > > > > > I915_FORMAT_MOD_Yf_TILED ?
> > > > > > > + stride / 2 : stride;
> > > > > >
> > > > > > The 2x part was rather well hidden in the spec. How do we deal with
> > > > > > cases when the Y stride is an odd number of tiles?
> > > > >
> > > > > It should be a round up division to take care of that scenario.
> > > >
> > > > That would stil lresult in a corrupted picture I think. So I was
> > > > thinking that we should just refuse to create NCV12 framebuffers with a
> > > > poorly aligned stride.
> > > >
> > > I added a check in intel_framebuffer_init() which should catch them:
> > > if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> > > DRM_DEBUG("y and uv subplanes have different pitches\n");
> > > return -EINVAL;
> > > }
> >
> > That won't catch the case I'm worried about. We would also need to make
> > sure pitches[1] is aligned to the UV tile width.
>
> If caller is following tile/row/pitch alignments properly for sub-planes of
> NV12 Yf buffer, above check will catch.
> But are you referring a case where userland isn't following tile/row size
> alignments properly? In that case, above may not catch. But
> isn't that is the case even with other FB formats if user land not
> following tile/row/pitch alignments?
We reject any attempt to create a framebuffer with a poorly aligned
stride.
Hmm. Actually I suppose we should just handle it in
intel_fb_stride_alignment(). Eg.:
case Yf:
if (cpp != 1 || pixel_format == DRM_FORMAT_NV12)
return 128;
else
return 64;
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-09 18:05 ` Ville Syrjälä
@ 2015-09-09 20:10 ` Konduru, Chandra
2015-09-09 20:40 ` Ville Syrjälä
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-09 20:10 UTC (permalink / raw
To: Ville Syrjälä
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Wednesday, September 09, 2015 11:06 AM
> To: Konduru, Chandra
> Cc: Syrjala, Ville; intel-gfx@lists.freedesktop.org; Vetter, Daniel
> Subject: Re: [Intel-gfx] [PATCH 11/15] drm/i915: Add NV12 to primary plane
> programming.
>
> On Wed, Sep 09, 2015 at 05:12:06PM +0000, Konduru, Chandra wrote:
> > > > > > > > + /* For tile-Yf, uv-subplane tile width is 2x of Y-
> > > subplane
> > > > > > > */
> > > > > > > > + aux_stride = fb->modifier[0] ==
> > > > > > > I915_FORMAT_MOD_Yf_TILED ?
> > > > > > > > + stride / 2 : stride;
> > > > > > >
> > > > > > > The 2x part was rather well hidden in the spec. How do we deal with
> > > > > > > cases when the Y stride is an odd number of tiles?
> > > > > >
> > > > > > It should be a round up division to take care of that scenario.
> > > > >
> > > > > That would stil lresult in a corrupted picture I think. So I was
> > > > > thinking that we should just refuse to create NCV12 framebuffers with a
> > > > > poorly aligned stride.
> > > > >
> > > > I added a check in intel_framebuffer_init() which should catch them:
> > > > if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> > > > DRM_DEBUG("y and uv subplanes have different pitches\n");
> > > > return -EINVAL;
> > > > }
> > >
> > > That won't catch the case I'm worried about. We would also need to make
> > > sure pitches[1] is aligned to the UV tile width.
> >
> > If caller is following tile/row/pitch alignments properly for sub-planes of
> > NV12 Yf buffer, above check will catch.
> > But are you referring a case where userland isn't following tile/row size
> > alignments properly? In that case, above may not catch. But
> > isn't that is the case even with other FB formats if user land not
> > following tile/row/pitch alignments?
>
> We reject any attempt to create a framebuffer with a poorly aligned
> stride.
>
> Hmm. Actually I suppose we should just handle it in
> intel_fb_stride_alignment(). Eg.:
>
> case Yf:
> if (cpp != 1 || pixel_format == DRM_FORMAT_NV12)
> return 128;
> else
> return 64;
This check is already there in intel_fb_stride_alignment()
which is called from intel_framebuffer_init(). But it always
does for 1st sub-plane which means does for Y.
It needs an update to pass a sub-plane (for UV) parameter,
and made another call for UV-plane alignment check.
Will this be ok?
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-09 20:10 ` Konduru, Chandra
@ 2015-09-09 20:40 ` Ville Syrjälä
2015-09-09 21:09 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-09 20:40 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
On Wed, Sep 09, 2015 at 08:10:30PM +0000, Konduru, Chandra wrote:
>
>
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Wednesday, September 09, 2015 11:06 AM
> > To: Konduru, Chandra
> > Cc: Syrjala, Ville; intel-gfx@lists.freedesktop.org; Vetter, Daniel
> > Subject: Re: [Intel-gfx] [PATCH 11/15] drm/i915: Add NV12 to primary plane
> > programming.
> >
> > On Wed, Sep 09, 2015 at 05:12:06PM +0000, Konduru, Chandra wrote:
> > > > > > > > > + /* For tile-Yf, uv-subplane tile width is 2x of Y-
> > > > subplane
> > > > > > > > */
> > > > > > > > > + aux_stride = fb->modifier[0] ==
> > > > > > > > I915_FORMAT_MOD_Yf_TILED ?
> > > > > > > > > + stride / 2 : stride;
> > > > > > > >
> > > > > > > > The 2x part was rather well hidden in the spec. How do we deal with
> > > > > > > > cases when the Y stride is an odd number of tiles?
> > > > > > >
> > > > > > > It should be a round up division to take care of that scenario.
> > > > > >
> > > > > > That would stil lresult in a corrupted picture I think. So I was
> > > > > > thinking that we should just refuse to create NCV12 framebuffers with a
> > > > > > poorly aligned stride.
> > > > > >
> > > > > I added a check in intel_framebuffer_init() which should catch them:
> > > > > if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> > > > > DRM_DEBUG("y and uv subplanes have different pitches\n");
> > > > > return -EINVAL;
> > > > > }
> > > >
> > > > That won't catch the case I'm worried about. We would also need to make
> > > > sure pitches[1] is aligned to the UV tile width.
> > >
> > > If caller is following tile/row/pitch alignments properly for sub-planes of
> > > NV12 Yf buffer, above check will catch.
> > > But are you referring a case where userland isn't following tile/row size
> > > alignments properly? In that case, above may not catch. But
> > > isn't that is the case even with other FB formats if user land not
> > > following tile/row/pitch alignments?
> >
> > We reject any attempt to create a framebuffer with a poorly aligned
> > stride.
> >
> > Hmm. Actually I suppose we should just handle it in
> > intel_fb_stride_alignment(). Eg.:
> >
> > case Yf:
> > if (cpp != 1 || pixel_format == DRM_FORMAT_NV12)
> > return 128;
> > else
> > return 64;
>
> This check is already there in intel_fb_stride_alignment()
> which is called from intel_framebuffer_init(). But it always
> does for 1st sub-plane which means does for Y.
> It needs an update to pass a sub-plane (for UV) parameter,
> and made another call for UV-plane alignment check.
> Will this be ok?
Yeah. You could in fact just have it loop over all the planes
and call it for each. Something like this perhaps:
for (i = 0; i < drm_format_num_planes(formt); i++) {
intel_fb_stride_alignment(modifier[i], format, i);
...
}
Or you could pass the cpp/bits_per_pixel instead of the plane index,
since that's the only thing for which you need the plane index within
the function.
I also started to wonder whether we should repeat most of the other
checks in intel_framebuffer_init() for each plane. But it's probably
just easier to check that handle, pitch and modifier matches for
both planes in the NV12 case. I think you actually missed the
modifier check. You just checked that modifier[1] is Yf, but that
leaves modifier[0] unchecked. I failed to notice it as well during
my review of the relevant patch.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-09 20:40 ` Ville Syrjälä
@ 2015-09-09 21:09 ` Konduru, Chandra
2015-09-09 22:27 ` Ville Syrjälä
0 siblings, 1 reply; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-09 21:09 UTC (permalink / raw
To: Ville Syrjälä
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> > > > > > > > > > + /* For tile-Yf, uv-subplane tile width is
> 2x of Y-
> > > > > subplane
> > > > > > > > > */
> > > > > > > > > > + aux_stride = fb->modifier[0] ==
> > > > > > > > > I915_FORMAT_MOD_Yf_TILED ?
> > > > > > > > > > + stride / 2 : stride;
> > > > > > > > >
> > > > > > > > > The 2x part was rather well hidden in the spec. How do we deal
> with
> > > > > > > > > cases when the Y stride is an odd number of tiles?
> > > > > > > >
> > > > > > > > It should be a round up division to take care of that scenario.
> > > > > > >
> > > > > > > That would stil lresult in a corrupted picture I think. So I was
> > > > > > > thinking that we should just refuse to create NCV12 framebuffers
> with a
> > > > > > > poorly aligned stride.
> > > > > > >
> > > > > > I added a check in intel_framebuffer_init() which should catch them:
> > > > > > if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> > > > > > DRM_DEBUG("y and uv subplanes have different pitches\n");
> > > > > > return -EINVAL;
> > > > > > }
> > > > >
> > > > > That won't catch the case I'm worried about. We would also need to
> make
> > > > > sure pitches[1] is aligned to the UV tile width.
> > > >
> > > > If caller is following tile/row/pitch alignments properly for sub-planes of
> > > > NV12 Yf buffer, above check will catch.
> > > > But are you referring a case where userland isn't following tile/row size
> > > > alignments properly? In that case, above may not catch. But
> > > > isn't that is the case even with other FB formats if user land not
> > > > following tile/row/pitch alignments?
> > >
> > > We reject any attempt to create a framebuffer with a poorly aligned
> > > stride.
> > >
> > > Hmm. Actually I suppose we should just handle it in
> > > intel_fb_stride_alignment(). Eg.:
> > >
> > > case Yf:
> > > if (cpp != 1 || pixel_format == DRM_FORMAT_NV12)
> > > return 128;
> > > else
> > > return 64;
> >
> > This check is already there in intel_fb_stride_alignment()
> > which is called from intel_framebuffer_init(). But it always
> > does for 1st sub-plane which means does for Y.
> > It needs an update to pass a sub-plane (for UV) parameter,
> > and made another call for UV-plane alignment check.
> > Will this be ok?
>
> Yeah. You could in fact just have it loop over all the planes
> and call it for each. Something like this perhaps:
> for (i = 0; i < drm_format_num_planes(formt); i++) {
> intel_fb_stride_alignment(modifier[i], format, i);
> ...
> }
I planned the same, looping for all subplanes.
>
> Or you could pass the cpp/bits_per_pixel instead of the plane index,
> since that's the only thing for which you need the plane index within
> the function.
>
> I also started to wonder whether we should repeat most of the other
> checks in intel_framebuffer_init() for each plane. But it's probably
> just easier to check that handle, pitch and modifier matches for
> both planes in the NV12 case. I think you actually missed the
> modifier check. You just checked that modifier[1] is Yf, but that
> leaves modifier[0] unchecked. I failed to notice it as well during
> my review of the relevant patch.
Added modifier check.
Made these changes to " drm/i915: Add NV12 support to intel_framebuffer_init"
patch. Also sending out updated WA patch to move to init clockgating.
Rest of your feedback is already addressed.
Before sending out these 2 patches, any other comments?
>
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-09 21:09 ` Konduru, Chandra
@ 2015-09-09 22:27 ` Ville Syrjälä
2015-09-09 23:31 ` Konduru, Chandra
0 siblings, 1 reply; 57+ messages in thread
From: Ville Syrjälä @ 2015-09-09 22:27 UTC (permalink / raw
To: Konduru, Chandra
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
On Wed, Sep 09, 2015 at 09:09:27PM +0000, Konduru, Chandra wrote:
>
> > > > > > > > > > > + /* For tile-Yf, uv-subplane tile width is
> > 2x of Y-
> > > > > > subplane
> > > > > > > > > > */
> > > > > > > > > > > + aux_stride = fb->modifier[0] ==
> > > > > > > > > > I915_FORMAT_MOD_Yf_TILED ?
> > > > > > > > > > > + stride / 2 : stride;
> > > > > > > > > >
> > > > > > > > > > The 2x part was rather well hidden in the spec. How do we deal
> > with
> > > > > > > > > > cases when the Y stride is an odd number of tiles?
> > > > > > > > >
> > > > > > > > > It should be a round up division to take care of that scenario.
> > > > > > > >
> > > > > > > > That would stil lresult in a corrupted picture I think. So I was
> > > > > > > > thinking that we should just refuse to create NCV12 framebuffers
> > with a
> > > > > > > > poorly aligned stride.
> > > > > > > >
> > > > > > > I added a check in intel_framebuffer_init() which should catch them:
> > > > > > > if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> > > > > > > DRM_DEBUG("y and uv subplanes have different pitches\n");
> > > > > > > return -EINVAL;
> > > > > > > }
> > > > > >
> > > > > > That won't catch the case I'm worried about. We would also need to
> > make
> > > > > > sure pitches[1] is aligned to the UV tile width.
> > > > >
> > > > > If caller is following tile/row/pitch alignments properly for sub-planes of
> > > > > NV12 Yf buffer, above check will catch.
> > > > > But are you referring a case where userland isn't following tile/row size
> > > > > alignments properly? In that case, above may not catch. But
> > > > > isn't that is the case even with other FB formats if user land not
> > > > > following tile/row/pitch alignments?
> > > >
> > > > We reject any attempt to create a framebuffer with a poorly aligned
> > > > stride.
> > > >
> > > > Hmm. Actually I suppose we should just handle it in
> > > > intel_fb_stride_alignment(). Eg.:
> > > >
> > > > case Yf:
> > > > if (cpp != 1 || pixel_format == DRM_FORMAT_NV12)
> > > > return 128;
> > > > else
> > > > return 64;
> > >
> > > This check is already there in intel_fb_stride_alignment()
> > > which is called from intel_framebuffer_init(). But it always
> > > does for 1st sub-plane which means does for Y.
> > > It needs an update to pass a sub-plane (for UV) parameter,
> > > and made another call for UV-plane alignment check.
> > > Will this be ok?
> >
> > Yeah. You could in fact just have it loop over all the planes
> > and call it for each. Something like this perhaps:
> > for (i = 0; i < drm_format_num_planes(formt); i++) {
> > intel_fb_stride_alignment(modifier[i], format, i);
> > ...
> > }
> I planned the same, looping for all subplanes.
>
> >
> > Or you could pass the cpp/bits_per_pixel instead of the plane index,
> > since that's the only thing for which you need the plane index within
> > the function.
> >
> > I also started to wonder whether we should repeat most of the other
> > checks in intel_framebuffer_init() for each plane. But it's probably
> > just easier to check that handle, pitch and modifier matches for
> > both planes in the NV12 case. I think you actually missed the
> > modifier check. You just checked that modifier[1] is Yf, but that
> > leaves modifier[0] unchecked. I failed to notice it as well during
> > my review of the relevant patch.
>
> Added modifier check.
> Made these changes to " drm/i915: Add NV12 support to intel_framebuffer_init"
> patch. Also sending out updated WA patch to move to init clockgating.
> Rest of your feedback is already addressed.
> Before sending out these 2 patches, any other comments?
Looking at what was said, I think we covered most open items
reasonably well, so fire away. I'll start going through the
updated patches tomorrow.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 11/15] drm/i915: Add NV12 to primary plane programming.
2015-09-09 22:27 ` Ville Syrjälä
@ 2015-09-09 23:31 ` Konduru, Chandra
0 siblings, 0 replies; 57+ messages in thread
From: Konduru, Chandra @ 2015-09-09 23:31 UTC (permalink / raw
To: Ville Syrjälä
Cc: Vetter, Daniel, intel-gfx@lists.freedesktop.org, Syrjala, Ville
> Looking at what was said, I think we covered most open items
> reasonably well, so fire away. I'll start going through the
> updated patches tomorrow.
>
Updates patches:
http://lists.freedesktop.org/archives/intel-gfx/2015-September/075235.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 57+ messages in thread
end of thread, other threads:[~2015-09-09 23:31 UTC | newest]
Thread overview: 57+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-20 1:02 [PATCH 00/15] drm/i915: Adding NV12 for skylake display Chandra Konduru
2015-08-20 1:02 ` [PATCH 01/15] drm/i915: Allocate min dbuf blocks per bspec Chandra Konduru
2015-09-04 8:17 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 02/15] drm/i915: In DBUF/WM calcs for 90/270, swap w & h Chandra Konduru
2015-09-04 8:31 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 03/15] drm/i915: Add register definitions for NV12 support Chandra Konduru
2015-09-04 8:40 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 04/15] drm/i915: Set scaler mode for NV12 Chandra Konduru
2015-09-04 8:53 ` Ville Syrjälä
2015-09-04 15:03 ` Daniel Vetter
2015-08-20 1:02 ` [PATCH 05/15] drm/i915: Stage scaler request for NV12 as src format Chandra Konduru
2015-09-04 10:17 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 06/15] drm/i915: Update format_is_yuv() to include NV12 Chandra Konduru
2015-09-04 10:17 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 07/15] drm/i915: Upscale scaler max scale for NV12 Chandra Konduru
2015-09-04 10:22 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 08/15] drm/i915: Add NV12 as supported format for primary plane Chandra Konduru
2015-08-26 8:40 ` Daniel Vetter
2015-08-27 1:40 ` Konduru, Chandra
2015-08-20 1:02 ` [PATCH 09/15] drm/i915: Add NV12 as supported format for sprite plane Chandra Konduru
2015-09-04 10:28 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init Chandra Konduru
2015-09-04 10:40 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 11/15] drm/i915: Add NV12 to primary plane programming Chandra Konduru
2015-09-04 11:09 ` Ville Syrjälä
2015-09-04 15:06 ` Daniel Vetter
2015-09-05 1:10 ` Konduru, Chandra
2015-09-05 14:59 ` Ville Syrjälä
2015-09-08 23:30 ` Konduru, Chandra
2015-09-09 11:41 ` Ville Syrjälä
2015-09-09 17:12 ` Konduru, Chandra
2015-09-09 18:05 ` Ville Syrjälä
2015-09-09 20:10 ` Konduru, Chandra
2015-09-09 20:40 ` Ville Syrjälä
2015-09-09 21:09 ` Konduru, Chandra
2015-09-09 22:27 ` Ville Syrjälä
2015-09-09 23:31 ` Konduru, Chandra
2015-08-20 1:02 ` [PATCH 12/15] drm/i915: Add NV12 to sprite " Chandra Konduru
2015-08-20 1:02 ` [PATCH 13/15] drm/i915: Set initial phase & trip for NV12 scaler Chandra Konduru
2015-09-04 11:15 ` Ville Syrjälä
2015-08-20 1:02 ` [PATCH 14/15] drm/i915: skl nv12 workarounds Chandra Konduru
2015-08-26 8:42 ` Daniel Vetter
2015-08-27 1:44 ` Konduru, Chandra
2015-09-02 8:02 ` Daniel Vetter
2015-09-03 18:33 ` Konduru, Chandra
2015-09-04 7:40 ` Daniel Vetter
2015-09-05 2:09 ` Konduru, Chandra
2015-09-04 11:26 ` Ville Syrjälä
2015-09-05 1:28 ` Konduru, Chandra
2015-09-05 14:52 ` Ville Syrjälä
2015-09-08 23:51 ` Konduru, Chandra
2015-09-09 11:46 ` Ville Syrjälä
2015-09-09 17:20 ` Konduru, Chandra
2015-08-20 1:02 ` [PATCH 15/15] drm/i915: Add 90/270 rotation for NV12 format Chandra Konduru
2015-09-04 11:30 ` Ville Syrjälä
2015-09-05 1:38 ` Konduru, Chandra
2015-09-05 14:48 ` Ville Syrjälä
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).