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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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