diff mbox

[2/2] drm/i915: Configure SKL+ scaler initial phase correctly

Message ID 20180521185613.5097-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä May 21, 2018, 6:56 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Set up the SKL+ scaler initial phase registers correctly. Otherwise
we start fetching the data from the center of the first pixel instead
of the top-left corner, which obviously then leads to right/bottom edges
replicating data excessively as the data runs out half a pixel too soon.

Cc: Vidya Srinivas <vidya.srinivas@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_sprite.c  | 26 +++++++++++++++++++++--
 4 files changed, 70 insertions(+), 2 deletions(-)

Comments

Vidya Srinivas May 31, 2018, 4:20 a.m. UTC | #1
Thank you.
Reviewed-By: Vidya Srinivas <vidya.srinivas@intel.com>


> -----Original Message-----

> From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]

> Sent: Tuesday, May 22, 2018 12:26 AM

> To: intel-gfx@lists.freedesktop.org

> Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Maarten Lankhorst

> <maarten.lankhorst@linux.intel.com>

> Subject: [PATCH 2/2] drm/i915: Configure SKL+ scaler initial phase correctly

> 

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 

> Set up the SKL+ scaler initial phase registers correctly. Otherwise we start

> fetching the data from the center of the first pixel instead of the top-left

> corner, which obviously then leads to right/bottom edges replicating data

> excessively as the data runs out half a pixel too soon.

> 

> Cc: Vidya Srinivas <vidya.srinivas@intel.com>

> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---

>  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++

>  drivers/gpu/drm/i915/intel_display.c | 41

> ++++++++++++++++++++++++++++++++++++

>  drivers/gpu/drm/i915/intel_drv.h     |  1 +

>  drivers/gpu/drm/i915/intel_sprite.c  | 26 +++++++++++++++++++++--

>  4 files changed, 70 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/i915_reg.h

> b/drivers/gpu/drm/i915/i915_reg.h index 196a0eb79272..094527403ede

> 100644

> --- a/drivers/gpu/drm/i915/i915_reg.h

> +++ b/drivers/gpu/drm/i915/i915_reg.h

> @@ -6774,6 +6774,10 @@ enum {

>  #define _PS_VPHASE_1B       0x68988

>  #define _PS_VPHASE_2B       0x68A88

>  #define _PS_VPHASE_1C       0x69188

> +#define  PS_Y_PHASE(x)		((x) << 16)

> +#define  PS_UV_RGB_PHASE(x)	((x) << 0)

> +#define   PS_PHASE_MASK	(0x7fff << 1) /* u2.13 */

> +#define   PS_PHASE_TRIP	(1 << 0)

> 

>  #define _PS_HPHASE_1A       0x68194

>  #define _PS_HPHASE_2A       0x68294

> diff --git a/drivers/gpu/drm/i915/intel_display.c

> b/drivers/gpu/drm/i915/intel_display.c

> index 42c1f4a56556..07185644a88a 100644

> --- a/drivers/gpu/drm/i915/intel_display.c

> +++ b/drivers/gpu/drm/i915/intel_display.c

> @@ -4729,6 +4729,39 @@ static void cpt_verify_modeset(struct

> drm_device *dev, int pipe)

>  	}

>  }

> 

> +/*

> + * The hardware phase 0.0 refers to the center of the pixel.

> + * We want to start from the top/left edge which is phase

> + * -0.5. That matches how the hardware calculates the scaling

> + * factors (from top-left of the first pixel to bottom-right

> + * of the last pixel, as opposed to the pixel centers).

> + *

> + * For 4:2:0 subsampled chroma planes we obviously have to

> + * adjust that so that the chroma sample position lands in

> + * the right spot.

> + *

> + * Note that for packed YCbCr 4:2:2 formats there is no way to

> + * control chroma siting. The hardware simply replicates the

> + * chroma samples for both of the luma samples, and thus we don't

> + * actually get the expected MPEG2 chroma siting convention :(

> + * The same behaviour is observed on pre-SKL platforms as well.

> + */

> +u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) {

> +	int phase = -0x8000;

> +	u16 trip = 0;

> +

> +	if (chroma_cosited)

> +		phase += (sub - 1) * 0x8000 / sub;

> +

> +	if (phase < 0)

> +		phase = 0x10000 + phase;

> +	else

> +		trip = PS_PHASE_TRIP;

> +

> +	return ((phase >> 2) & PS_PHASE_MASK) | trip; }

> +

>  static int

>  skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,

>  		  unsigned int scaler_user, int *scaler_id, @@ -4928,14

> +4961,22 @@ static void skylake_pfit_enable(struct intel_crtc *crtc)

>  		&crtc->config->scaler_state;

> 

>  	if (crtc->config->pch_pfit.enabled) {

> +		u16 uv_rgb_hphase, uv_rgb_vphase;

>  		int id;

> 

>  		if (WARN_ON(crtc->config->scaler_state.scaler_id < 0))

>  			return;

> 

> +		uv_rgb_hphase = skl_scaler_calc_phase(1, false);

> +		uv_rgb_vphase = skl_scaler_calc_phase(1, false);

> +

>  		id = scaler_state->scaler_id;

>  		I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN |

>  			PS_FILTER_MEDIUM | scaler_state-

> >scalers[id].mode);

> +		I915_WRITE_FW(SKL_PS_VPHASE(pipe, id),

> +			      PS_Y_PHASE(0) |

> PS_UV_RGB_PHASE(uv_rgb_vphase));

> +		I915_WRITE_FW(SKL_PS_HPHASE(pipe, id),

> +			      PS_Y_PHASE(0) |

> PS_UV_RGB_PHASE(uv_rgb_hphase));

>  		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config-

> >pch_pfit.pos);

>  		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config-

> >pch_pfit.size);

>  	}

> diff --git a/drivers/gpu/drm/i915/intel_drv.h

> b/drivers/gpu/drm/i915/intel_drv.h

> index 42a59b7fd736..0f1dbfcf8538 100644

> --- a/drivers/gpu/drm/i915/intel_drv.h

> +++ b/drivers/gpu/drm/i915/intel_drv.h

> @@ -1621,6 +1621,7 @@ void intel_mode_from_pipe_config(struct

> drm_display_mode *mode,  void intel_crtc_arm_fifo_underrun(struct

> intel_crtc *crtc,

>  				  struct intel_crtc_state *crtc_state);

> 

> +u16 skl_scaler_calc_phase(int sub, bool chroma_center);

>  int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);  int

> skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,

>  		  uint32_t pixel_format);

> diff --git a/drivers/gpu/drm/i915/intel_sprite.c

> b/drivers/gpu/drm/i915/intel_sprite.c

> index 214cc730642c..58522441253d 100644

> --- a/drivers/gpu/drm/i915/intel_sprite.c

> +++ b/drivers/gpu/drm/i915/intel_sprite.c

> @@ -284,13 +284,35 @@ skl_update_plane(struct intel_plane *plane,

>  	/* program plane scaler */

>  	if (plane_state->scaler_id >= 0) {

>  		int scaler_id = plane_state->scaler_id;

> -		const struct intel_scaler *scaler;

> +		const struct intel_scaler *scaler =

> +			&crtc_state->scaler_state.scalers[scaler_id];

> +		u16 y_hphase, uv_rgb_hphase;

> +		u16 y_vphase, uv_rgb_vphase;

> +

> +		/* TODO: handle sub-pixel coordinates */

> +		if (fb->format->format == DRM_FORMAT_NV12) {

> +			y_hphase = skl_scaler_calc_phase(1, false);

> +			y_vphase = skl_scaler_calc_phase(1, false);

> +

> +			/* MPEG2 chroma siting convention */

> +			uv_rgb_hphase = skl_scaler_calc_phase(2, true);

> +			uv_rgb_vphase = skl_scaler_calc_phase(2, false);

> +		} else {

> +			/* not used */

> +			y_hphase = 0;

> +			y_vphase = 0;

> 

> -		scaler = &crtc_state->scaler_state.scalers[scaler_id];

> +			uv_rgb_hphase = skl_scaler_calc_phase(1, false);

> +			uv_rgb_vphase = skl_scaler_calc_phase(1, false);

> +		}

> 

>  		I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id),

>  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |

> scaler->mode);

>  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);

> +		I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id),

> +			      PS_Y_PHASE(y_vphase) |

> PS_UV_RGB_PHASE(uv_rgb_vphase));

> +		I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id),

> +			      PS_Y_PHASE(y_hphase) |

> PS_UV_RGB_PHASE(uv_rgb_hphase));

>  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x

> << 16) | crtc_y);

>  		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),

>  			      ((crtc_w + 1) << 16)|(crtc_h + 1));

> --

> 2.16.1
Ville Syrjälä June 1, 2018, 3:52 p.m. UTC | #2
On Thu, May 31, 2018 at 04:20:24AM +0000, Srinivas, Vidya wrote:
> Thank you.
> Reviewed-By: Vidya Srinivas <vidya.srinivas@intel.com>

Thanks. Series pushed to dinq.

> 
> > -----Original Message-----
> > From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
> > Sent: Tuesday, May 22, 2018 12:26 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Srinivas, Vidya <vidya.srinivas@intel.com>; Maarten Lankhorst
> > <maarten.lankhorst@linux.intel.com>
> > Subject: [PATCH 2/2] drm/i915: Configure SKL+ scaler initial phase correctly
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Set up the SKL+ scaler initial phase registers correctly. Otherwise we start
> > fetching the data from the center of the first pixel instead of the top-left
> > corner, which obviously then leads to right/bottom edges replicating data
> > excessively as the data runs out half a pixel too soon.
> > 
> > Cc: Vidya Srinivas <vidya.srinivas@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h      |  4 ++++
> >  drivers/gpu/drm/i915/intel_display.c | 41
> > ++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  1 +
> >  drivers/gpu/drm/i915/intel_sprite.c  | 26 +++++++++++++++++++++--
> >  4 files changed, 70 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 196a0eb79272..094527403ede
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6774,6 +6774,10 @@ enum {
> >  #define _PS_VPHASE_1B       0x68988
> >  #define _PS_VPHASE_2B       0x68A88
> >  #define _PS_VPHASE_1C       0x69188
> > +#define  PS_Y_PHASE(x)		((x) << 16)
> > +#define  PS_UV_RGB_PHASE(x)	((x) << 0)
> > +#define   PS_PHASE_MASK	(0x7fff << 1) /* u2.13 */
> > +#define   PS_PHASE_TRIP	(1 << 0)
> > 
> >  #define _PS_HPHASE_1A       0x68194
> >  #define _PS_HPHASE_2A       0x68294
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 42c1f4a56556..07185644a88a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4729,6 +4729,39 @@ static void cpt_verify_modeset(struct
> > drm_device *dev, int pipe)
> >  	}
> >  }
> > 
> > +/*
> > + * The hardware phase 0.0 refers to the center of the pixel.
> > + * We want to start from the top/left edge which is phase
> > + * -0.5. That matches how the hardware calculates the scaling
> > + * factors (from top-left of the first pixel to bottom-right
> > + * of the last pixel, as opposed to the pixel centers).
> > + *
> > + * For 4:2:0 subsampled chroma planes we obviously have to
> > + * adjust that so that the chroma sample position lands in
> > + * the right spot.
> > + *
> > + * Note that for packed YCbCr 4:2:2 formats there is no way to
> > + * control chroma siting. The hardware simply replicates the
> > + * chroma samples for both of the luma samples, and thus we don't
> > + * actually get the expected MPEG2 chroma siting convention :(
> > + * The same behaviour is observed on pre-SKL platforms as well.
> > + */
> > +u16 skl_scaler_calc_phase(int sub, bool chroma_cosited) {
> > +	int phase = -0x8000;
> > +	u16 trip = 0;
> > +
> > +	if (chroma_cosited)
> > +		phase += (sub - 1) * 0x8000 / sub;
> > +
> > +	if (phase < 0)
> > +		phase = 0x10000 + phase;
> > +	else
> > +		trip = PS_PHASE_TRIP;
> > +
> > +	return ((phase >> 2) & PS_PHASE_MASK) | trip; }
> > +
> >  static int
> >  skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
> >  		  unsigned int scaler_user, int *scaler_id, @@ -4928,14
> > +4961,22 @@ static void skylake_pfit_enable(struct intel_crtc *crtc)
> >  		&crtc->config->scaler_state;
> > 
> >  	if (crtc->config->pch_pfit.enabled) {
> > +		u16 uv_rgb_hphase, uv_rgb_vphase;
> >  		int id;
> > 
> >  		if (WARN_ON(crtc->config->scaler_state.scaler_id < 0))
> >  			return;
> > 
> > +		uv_rgb_hphase = skl_scaler_calc_phase(1, false);
> > +		uv_rgb_vphase = skl_scaler_calc_phase(1, false);
> > +
> >  		id = scaler_state->scaler_id;
> >  		I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
> >  			PS_FILTER_MEDIUM | scaler_state-
> > >scalers[id].mode);
> > +		I915_WRITE_FW(SKL_PS_VPHASE(pipe, id),
> > +			      PS_Y_PHASE(0) |
> > PS_UV_RGB_PHASE(uv_rgb_vphase));
> > +		I915_WRITE_FW(SKL_PS_HPHASE(pipe, id),
> > +			      PS_Y_PHASE(0) |
> > PS_UV_RGB_PHASE(uv_rgb_hphase));
> >  		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config-
> > >pch_pfit.pos);
> >  		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config-
> > >pch_pfit.size);
> >  	}
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 42a59b7fd736..0f1dbfcf8538 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1621,6 +1621,7 @@ void intel_mode_from_pipe_config(struct
> > drm_display_mode *mode,  void intel_crtc_arm_fifo_underrun(struct
> > intel_crtc *crtc,
> >  				  struct intel_crtc_state *crtc_state);
> > 
> > +u16 skl_scaler_calc_phase(int sub, bool chroma_center);
> >  int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);  int
> > skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> >  		  uint32_t pixel_format);
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 214cc730642c..58522441253d 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -284,13 +284,35 @@ skl_update_plane(struct intel_plane *plane,
> >  	/* program plane scaler */
> >  	if (plane_state->scaler_id >= 0) {
> >  		int scaler_id = plane_state->scaler_id;
> > -		const struct intel_scaler *scaler;
> > +		const struct intel_scaler *scaler =
> > +			&crtc_state->scaler_state.scalers[scaler_id];
> > +		u16 y_hphase, uv_rgb_hphase;
> > +		u16 y_vphase, uv_rgb_vphase;
> > +
> > +		/* TODO: handle sub-pixel coordinates */
> > +		if (fb->format->format == DRM_FORMAT_NV12) {
> > +			y_hphase = skl_scaler_calc_phase(1, false);
> > +			y_vphase = skl_scaler_calc_phase(1, false);
> > +
> > +			/* MPEG2 chroma siting convention */
> > +			uv_rgb_hphase = skl_scaler_calc_phase(2, true);
> > +			uv_rgb_vphase = skl_scaler_calc_phase(2, false);
> > +		} else {
> > +			/* not used */
> > +			y_hphase = 0;
> > +			y_vphase = 0;
> > 
> > -		scaler = &crtc_state->scaler_state.scalers[scaler_id];
> > +			uv_rgb_hphase = skl_scaler_calc_phase(1, false);
> > +			uv_rgb_vphase = skl_scaler_calc_phase(1, false);
> > +		}
> > 
> >  		I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id),
> >  			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> > scaler->mode);
> >  		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
> > +		I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id),
> > +			      PS_Y_PHASE(y_vphase) |
> > PS_UV_RGB_PHASE(uv_rgb_vphase));
> > +		I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id),
> > +			      PS_Y_PHASE(y_hphase) |
> > PS_UV_RGB_PHASE(uv_rgb_hphase));
> >  		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> > << 16) | crtc_y);
> >  		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
> >  			      ((crtc_w + 1) << 16)|(crtc_h + 1));
> > --
> > 2.16.1
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 196a0eb79272..094527403ede 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6774,6 +6774,10 @@  enum {
 #define _PS_VPHASE_1B       0x68988
 #define _PS_VPHASE_2B       0x68A88
 #define _PS_VPHASE_1C       0x69188
+#define  PS_Y_PHASE(x)		((x) << 16)
+#define  PS_UV_RGB_PHASE(x)	((x) << 0)
+#define   PS_PHASE_MASK	(0x7fff << 1) /* u2.13 */
+#define   PS_PHASE_TRIP	(1 << 0)
 
 #define _PS_HPHASE_1A       0x68194
 #define _PS_HPHASE_2A       0x68294
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 42c1f4a56556..07185644a88a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4729,6 +4729,39 @@  static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 	}
 }
 
+/*
+ * The hardware phase 0.0 refers to the center of the pixel.
+ * We want to start from the top/left edge which is phase
+ * -0.5. That matches how the hardware calculates the scaling
+ * factors (from top-left of the first pixel to bottom-right
+ * of the last pixel, as opposed to the pixel centers).
+ *
+ * For 4:2:0 subsampled chroma planes we obviously have to
+ * adjust that so that the chroma sample position lands in
+ * the right spot.
+ *
+ * Note that for packed YCbCr 4:2:2 formats there is no way to
+ * control chroma siting. The hardware simply replicates the
+ * chroma samples for both of the luma samples, and thus we don't
+ * actually get the expected MPEG2 chroma siting convention :(
+ * The same behaviour is observed on pre-SKL platforms as well.
+ */
+u16 skl_scaler_calc_phase(int sub, bool chroma_cosited)
+{
+	int phase = -0x8000;
+	u16 trip = 0;
+
+	if (chroma_cosited)
+		phase += (sub - 1) * 0x8000 / sub;
+
+	if (phase < 0)
+		phase = 0x10000 + phase;
+	else
+		trip = PS_PHASE_TRIP;
+
+	return ((phase >> 2) & PS_PHASE_MASK) | trip;
+}
+
 static int
 skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
 		  unsigned int scaler_user, int *scaler_id,
@@ -4928,14 +4961,22 @@  static void skylake_pfit_enable(struct intel_crtc *crtc)
 		&crtc->config->scaler_state;
 
 	if (crtc->config->pch_pfit.enabled) {
+		u16 uv_rgb_hphase, uv_rgb_vphase;
 		int id;
 
 		if (WARN_ON(crtc->config->scaler_state.scaler_id < 0))
 			return;
 
+		uv_rgb_hphase = skl_scaler_calc_phase(1, false);
+		uv_rgb_vphase = skl_scaler_calc_phase(1, false);
+
 		id = scaler_state->scaler_id;
 		I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN |
 			PS_FILTER_MEDIUM | scaler_state->scalers[id].mode);
+		I915_WRITE_FW(SKL_PS_VPHASE(pipe, id),
+			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_vphase));
+		I915_WRITE_FW(SKL_PS_HPHASE(pipe, id),
+			      PS_Y_PHASE(0) | PS_UV_RGB_PHASE(uv_rgb_hphase));
 		I915_WRITE(SKL_PS_WIN_POS(pipe, id), crtc->config->pch_pfit.pos);
 		I915_WRITE(SKL_PS_WIN_SZ(pipe, id), crtc->config->pch_pfit.size);
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 42a59b7fd736..0f1dbfcf8538 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1621,6 +1621,7 @@  void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 void intel_crtc_arm_fifo_underrun(struct intel_crtc *crtc,
 				  struct intel_crtc_state *crtc_state);
 
+u16 skl_scaler_calc_phase(int sub, bool chroma_center);
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		  uint32_t pixel_format);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 214cc730642c..58522441253d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -284,13 +284,35 @@  skl_update_plane(struct intel_plane *plane,
 	/* program plane scaler */
 	if (plane_state->scaler_id >= 0) {
 		int scaler_id = plane_state->scaler_id;
-		const struct intel_scaler *scaler;
+		const struct intel_scaler *scaler =
+			&crtc_state->scaler_state.scalers[scaler_id];
+		u16 y_hphase, uv_rgb_hphase;
+		u16 y_vphase, uv_rgb_vphase;
+
+		/* TODO: handle sub-pixel coordinates */
+		if (fb->format->format == DRM_FORMAT_NV12) {
+			y_hphase = skl_scaler_calc_phase(1, false);
+			y_vphase = skl_scaler_calc_phase(1, false);
+
+			/* MPEG2 chroma siting convention */
+			uv_rgb_hphase = skl_scaler_calc_phase(2, true);
+			uv_rgb_vphase = skl_scaler_calc_phase(2, false);
+		} else {
+			/* not used */
+			y_hphase = 0;
+			y_vphase = 0;
 
-		scaler = &crtc_state->scaler_state.scalers[scaler_id];
+			uv_rgb_hphase = skl_scaler_calc_phase(1, false);
+			uv_rgb_vphase = skl_scaler_calc_phase(1, false);
+		}
 
 		I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id),
 			      PS_SCALER_EN | PS_PLANE_SEL(plane_id) | scaler->mode);
 		I915_WRITE_FW(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
+		I915_WRITE_FW(SKL_PS_VPHASE(pipe, scaler_id),
+			      PS_Y_PHASE(y_vphase) | PS_UV_RGB_PHASE(uv_rgb_vphase));
+		I915_WRITE_FW(SKL_PS_HPHASE(pipe, scaler_id),
+			      PS_Y_PHASE(y_hphase) | PS_UV_RGB_PHASE(uv_rgb_hphase));
 		I915_WRITE_FW(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
 		I915_WRITE_FW(SKL_PS_WIN_SZ(pipe, scaler_id),
 			      ((crtc_w + 1) << 16)|(crtc_h + 1));