On Wed, Aug 10, 2016 at 12:23:20PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> With NV12 we have two color planes to deal with so we must compute the
> surface and x/y offsets for the second plane as well.
>
> What makes this a bit nasty is that the hardware expects the surface
> offset to be specified as a distance from the main surface offset.
> What's worse, the distance must be non-negative (no neat wraparound or
> anything). So we must make sure that the main surface offset is always
> less or equal to the AUX surface offset. We do that by computing the AUX
> offset first and the main surface offset second. If the main surface
> offset ends up being above the AUX offset, we just push it down as far
> as is required while still maintaining the required alignment etc.
>
> Fortunately the AUX offset only reuqires 4K alignment, so we don't need
> to do any of the backwards searching for an acceptable offset that we
> must do for the main surface. And X tiled + NV12 isn't a supported
> combination anyway.
>
> Note that this just computes aux surface offsets, we do not yet program
> them into the actual hardware registers, and hence we can't yet expose
> NV12.
>
> v2: Rebase due to drm_plane_state src/dst rects
> s/TODO.../something else/ in the commit message/ (Daniel)
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 62 ++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_drv.h | 4 +++
> 2 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b719c07599fd..f8487bcdb271 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2463,8 +2463,14 @@ u32 intel_compute_tile_offset(int *x, int *y,
> const struct drm_i915_private *dev_priv = to_i915(state->base.plane->dev);
> const struct drm_framebuffer *fb = state->base.fb;
> unsigned int rotation = state->base.rotation;
> - u32 alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
> int pitch = intel_fb_pitch(fb, plane, rotation);
> + u32 alignment;
> +
> + /* AUX_DIST needs only 4K alignment */
> + if (fb->pixel_format == DRM_FORMAT_NV12 && plane == 1)
> + alignment = 4096;
> + else
> + alignment = intel_surf_alignment(dev_priv, fb->modifier[plane]);
>
> return _intel_compute_tile_offset(dev_priv, x, y, fb, plane, pitch,
> rotation, alignment);
> @@ -2895,7 +2901,7 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> int h = drm_rect_height(&plane_state->base.src) >> 16;
> int max_width = skl_max_plane_width(fb, 0, rotation);
> int max_height = 4096;
> - u32 alignment, offset;
> + u32 alignment, offset, aux_offset = plane_state->aux.offset;
>
> if (w > max_width || h > max_height) {
> DRM_DEBUG_KMS("requested Y/RGB source size %dx%d too big (limit %dx%d)\n",
> @@ -2909,6 +2915,15 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> alignment = intel_surf_alignment(dev_priv, fb->modifier[0]);
>
> /*
> + * AUX surface offset is specified as the distance from the
> + * main surface offset, and it must be non-negative. Make
> + * sure that is what we will get.
> + */
> + if (offset > aux_offset)
> + offset = intel_adjust_tile_offset(&x, &y, plane_state, 0,
> + offset, aux_offset & ~(alignment - 1));
Note to self: Need to make sure that multiplanar fbs all reference the
same underlying bo. But that code isn't yet added to
intel_framebuffer_init().
> +
> + /*
> * When using an X-tiled surface, the plane blows up
> * if the x offset + width exceed the stride.
> *
> @@ -2935,6 +2950,35 @@ static int skl_check_main_surface(struct intel_plane_state *plane_state)
> return 0;
> }
>
> +static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
> +{
> + const struct drm_framebuffer *fb = plane_state->base.fb;
> + unsigned int rotation = plane_state->base.rotation;
> + int max_width = skl_max_plane_width(fb, 1, rotation);
> + int max_height = 4096;
> + int x = plane_state->base.src.x1 >> 17;
> + int y = plane_state->base.src.y1 >> 17;
> + int w = drm_rect_width(&plane_state->base.src) >> 17;
> + int h = drm_rect_height(&plane_state->base.src) >> 17;
> + u32 offset;
> +
> + intel_add_fb_offsets(&x, &y, plane_state, 1);
> + offset = intel_compute_tile_offset(&x, &y, plane_state, 1);
> +
> + /* FIXME not quite sure how/if these apply to the chroma plane */
> + if (w > max_width || h > max_height) {
> + DRM_DEBUG_KMS("CbCr source size %dx%d too big (limit %dx%d)\n",
> + w, h, max_width, max_height);
> + return -EINVAL;
> + }
We also need to check pitch of the 2nd plane, but I guess that's done at
fb_init time, not at atomic_check time. Looks all reasonable as-is.
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> +
> + plane_state->aux.offset = offset;
> + plane_state->aux.x = x;
> + plane_state->aux.y = y;
> +
> + return 0;
> +}
> +
> int skl_check_plane_surface(struct intel_plane_state *plane_state)
> {
> const struct drm_framebuffer *fb = plane_state->base.fb;
> @@ -2946,6 +2990,20 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
> drm_rect_rotate(&plane_state->base.src,
> fb->width, fb->height, BIT(DRM_ROTATE_270));
>
> + /*
> + * Handle the AUX surface first since
> + * the main surface setup depends on it.
> + */
> + if (fb->pixel_format == DRM_FORMAT_NV12) {
> + ret = skl_check_nv12_aux_surface(plane_state);
> + if (ret)
> + return ret;
> + } else {
> + plane_state->aux.offset = ~0xfff;
> + plane_state->aux.x = 0;
> + plane_state->aux.y = 0;
> + }
> +
> ret = skl_check_main_surface(plane_state);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2fde13833fcb..df8e7514c08c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -354,6 +354,10 @@ struct intel_plane_state {
> u32 offset;
> int x, y;
> } main;
> + struct {
> + u32 offset;
> + int x, y;
> + } aux;
>
> /*
> * scaler_id
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
