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

Ville Syrjälä
2015-09-29T18:37:04Z

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

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

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

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

o o o o

# o # o

o o o o

# o # o

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

o o o o
x x
o o o o

0 0 0 0
x x
0 0 0 0

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

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

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

--
Ville Syrjälä
Intel OTC