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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx