| Message ID | 1431117622-6469-1-git-send-email-chandra.konduru@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 05/08/2015 09:40 PM, 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) > > Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> > Testcase: igt/kms_nv12 > --- > drivers/gpu/drm/i915/intel_atomic_plane.c | 2 ++ > drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > index 86ba4b2..119439d 100644 > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -185,10 +185,12 @@ static int intel_plane_atomic_check(struct drm_plane *plane, > * 90/270 is not allowed with RGB64 16:16:16:16, > * RGB 16-bit 5:6:5, and Indexed 8-bit. > * TBD: Add RGB64 case once its added in supported format list. > + * TBD: Remove NV12, once its 90/270 remapping is supported > */ > switch (state->fb->pixel_format) { > case DRM_FORMAT_C8: > case DRM_FORMAT_RGB565: > + case DRM_FORMAT_NV12: > DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n", > drm_get_format_name(state->fb->pixel_format)); > return -EINVAL; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3ae646e..943a835 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3015,6 +3015,9 @@ u32 skl_plane_ctl_format(uint32_t pixel_format) > case DRM_FORMAT_VYUY: > plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY; > break; > + case DRM_FORMAT_NV12: > + plane_ctl_format = PLANE_CTL_FORMAT_NV12; > + break; > default: > BUG(); > } > @@ -3085,6 +3088,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); > > @@ -3141,11 +3146,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; > > @@ -3153,11 +3181,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); > @@ -3166,6 +3197,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); I've hit this WARN_ON with kms_rotation_crc by just changing the format of frame buffers it creates to NV12 (so even before any rotation happens). Small snippet: [ 906.408194] [drm:skl_update_scaler_users] PLANE:17 staged scaling request for 3200x1800->3200x1800 crtc_state = ffff880037348800 scaler_users = 0x1 [ 906.436759] ------------[ cut here ]------------ [ 906.442107] WARNING: CPU: 0 PID: 1709 at drivers/gpu/drm/i915/intel_display.c:3225 skylake_update_primary_plane+0x678/0x6e0 [i915]() [ 906.455620] WARN_ON(fb->pixel_format == DRM_FORMAT_NV12) [ 906.461451] Modules linked in: coretemp i915 drm_kms_helper drm i2c_algo_bit i2c_hid video pinctrl_sunrisepoint pinctrl_intel acpi_pad nls_iso8859_1 hid_generic usbhid hid e1000e ptp psmouse ahci pps_core libahci [ 906.483472] CPU: 0 PID: 1709 Comm: kms_rotation_cr Tainted: G U W 4.1.0-rc3-150511+ #12 [ 906.493638] Hardware name: Intel Corporation Skylake Client platform/Skylake Y LPDDR3 RVP3, BIOS SKLSE2R1.86C.X070.R01.1501282110 01/28/2015 [ 906.507938] 0000000000000c99 ffff88009bc979a8 ffffffff8161283d 0000000000000007 [ 906.516410] ffff88009bc979f8 ffff88009bc979e8 ffffffff81050062 ffff88009acd0000 [ 906.524874] ffff88009acd0000 ffff8800372356c0 0000000000000000 00000000c1802000 [ 906.533341] Call Trace: [ 906.536135] [<ffffffff8161283d>] dump_stack+0x4c/0x6e [ 906.541979] [<ffffffff81050062>] warn_slowpath_common+0xb2/0xe0 [ 906.548802] [<ffffffff81050146>] warn_slowpath_fmt+0x46/0x50 [ 906.555408] [<ffffffffa0242cf8>] skylake_update_primary_plane+0x678/0x6e0 [i915] [ 906.563980] [<ffffffffa023cca3>] intel_commit_primary_plane+0x93/0xb0 [i915] [ 906.572158] [<ffffffffa025fa49>] intel_plane_atomic_update+0x19/0x20 [i915] [ 906.580180] [<ffffffffa01b026a>] drm_atomic_helper_commit_planes+0x17a/0x210 [drm_kms_helper] [ 906.590029] [<ffffffffa0245de3>] __intel_set_mode+0xbf3/0xcd0 [i915] [ 906.597418] [<ffffffffa024bdae>] intel_crtc_set_config+0x4ee/0x650 [i915] [ 906.605277] [<ffffffffa014b549>] drm_mode_set_config_internal+0x69/0x120 [drm] [ 906.613618] [<ffffffffa0150b48>] drm_mode_setcrtc+0x558/0x650 [drm] [ 906.620856] [<ffffffffa0140f19>] drm_ioctl+0x3f9/0x650 [drm] [ 906.627419] [<ffffffffa01505f0>] ? drm_mode_setplane+0x200/0x200 [drm] [ 906.634939] [<ffffffff8116a1f3>] do_vfs_ioctl+0x543/0x5a0 [ 906.641170] [<ffffffff810a8576>] ? rcu_eqs_exit+0x96/0xb0 [ 906.647408] [<ffffffff81095f2d>] ? trace_hardirqs_on+0xd/0x10 [ 906.654038] [<ffffffff81175cd7>] ? __fget_light+0x57/0xa0 [ 906.660271] [<ffffffff8116a29c>] SyS_ioctl+0x4c/0x90 [ 906.666031] [<ffffffff8161aed7>] system_call_fastpath+0x12/0x6f [ 906.672854] ---[ end trace 599568c0e80eee2a ]--- [ 906.678235] [drm:intel_pipe_update_end [i915]] *ERROR* Atomic update failure on pipe A (start=10907 end=10921) [ 906.679305] [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe A FIFO underrun Machine even hard hangs in the process. Either way, it is probably bad that userspace can hit this WARN_ON just by trying to display a NV12 frame buffer. Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 86ba4b2..119439d 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -185,10 +185,12 @@ static int intel_plane_atomic_check(struct drm_plane *plane, * 90/270 is not allowed with RGB64 16:16:16:16, * RGB 16-bit 5:6:5, and Indexed 8-bit. * TBD: Add RGB64 case once its added in supported format list. + * TBD: Remove NV12, once its 90/270 remapping is supported */ switch (state->fb->pixel_format) { case DRM_FORMAT_C8: case DRM_FORMAT_RGB565: + case DRM_FORMAT_NV12: DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n", drm_get_format_name(state->fb->pixel_format)); return -EINVAL; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3ae646e..943a835 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3015,6 +3015,9 @@ u32 skl_plane_ctl_format(uint32_t pixel_format) case DRM_FORMAT_VYUY: plane_ctl_format = PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY; break; + case DRM_FORMAT_NV12: + plane_ctl_format = PLANE_CTL_FORMAT_NV12; + break; default: BUG(); } @@ -3085,6 +3088,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); @@ -3141,11 +3146,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; @@ -3153,11 +3181,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); @@ -3166,6 +3197,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); } @@ -13090,6 +13122,12 @@ intel_check_primary_plane(struct drm_plane *plane, intel_crtc->atomic.update_wm = true; } + /* Adjust (macro)pixel boundary */ + if (fb && format_is_yuv(fb->pixel_format)) { + src->x1 &= ~0x10000; + src->x2 &= ~0x10000; + } + if (INTEL_INFO(dev)->gen >= 9) { ret = skl_update_scaler_users(intel_crtc, crtc_state, to_intel_plane(plane), state, 0);
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) Signed-off-by: Chandra Konduru <chandra.konduru@intel.com> Testcase: igt/kms_nv12 --- drivers/gpu/drm/i915/intel_atomic_plane.c | 2 ++ drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+)