drm/i915: Account for scale factor when calculating initial phase

To get the initial phase correct we need to account for the scale
factor as well. I forgot this initially and was mostly looking at
heavily upscaled content where the minor difference between -0.5
and the proper initial phase was not readily apparent.

And let's toss in a comment that tries to explain the formula
a little bit.

v2: The initial phase upper limit is 1.5, not 24.0!

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: 0a59952b24e2 ("drm/i915: Configure SKL+ scaler initial phase correctly")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20181029181820.21956-1-ville.syrjala@linux.intel.com
Tested-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Tested-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> #irc
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> #irc

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 940577f8c041ab958d27ae9b14ad806e12c51218..e73638fffe32d8185a4d18ddaf32a93799f4547c 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4777,8 +4777,31 @@ static void cpt_verify_modeset(struct drm_device *dev, int pipe)
  * 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.
+ *
+ * Theory behind the formula (note that we ignore sub-pixel
+ * source coordinates):
+ * s = source sample position
+ * d = destination sample position
+ *
+ * Downscaling 4:1:
+ * -0.5
+ * | 0.0
+ * | |     1.5 (initial phase)
+ * | |     |
+ * v v     v
+ * | s | s | s | s |
+ * |       d       |
+ *
+ * Upscaling 1:4:
+ * -0.5
+ * | -0.375 (initial phase)
+ * | |     0.0
+ * | |     |
+ * v v     v
+ * |       s       |
+ * | d | d | d | d |
  */
-u16 skl_scaler_calc_phase(int sub, bool chroma_cosited)
+u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_cosited)
 {
        int phase = -0x8000;
        u16 trip = 0;
@@ -4786,6 +4809,15 @@ u16 skl_scaler_calc_phase(int sub, bool chroma_cosited)
        if (chroma_cosited)
                phase += (sub - 1) * 0x8000 / sub;
 
+       phase += scale / (2 * sub);
+
+       /*
+        * Hardware initial phase limited to [-0.5:1.5].
+        * Since the max hardware scale factor is 3.0, we
+        * should never actually excdeed 1.0 here.
+        */
+       WARN_ON(phase < -0x8000 || phase > 0x18000);
+
        if (phase < 0)
                phase = 0x10000 + phase;
        else
@@ -4994,13 +5026,20 @@ static void skylake_pfit_enable(const struct intel_crtc_state *crtc_state)
 
        if (crtc_state->pch_pfit.enabled) {
                u16 uv_rgb_hphase, uv_rgb_vphase;
+               int pfit_w, pfit_h, hscale, vscale;
                int id;
 
                if (WARN_ON(crtc_state->scaler_state.scaler_id < 0))
                        return;
 
-               uv_rgb_hphase = skl_scaler_calc_phase(1, false);
-               uv_rgb_vphase = skl_scaler_calc_phase(1, false);
+               pfit_w = (crtc_state->pch_pfit.size >> 16) & 0xFFFF;
+               pfit_h = crtc_state->pch_pfit.size & 0xFFFF;
+
+               hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
+               vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
+
+               uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
+               uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
 
                id = scaler_state->scaler_id;
                I915_WRITE(SKL_PS_CTRL(pipe, id), PS_SCALER_EN |

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cc7fab2b61f4b18a5fbd582fb944e025af90d351..18b419f7f7fe85b2d87446b64ff526cfa63dedad 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1720,7 +1720,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);
+u16 skl_scaler_calc_phase(int sub, int scale, bool chroma_center);
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 int skl_max_scale(const struct intel_crtc_state *crtc_state,
                  u32 pixel_format);

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index a38270b6e6d4381ff2f2fa2fae5c4c2b317f754f..5e0f7b575a50e8f5109700532124b6fff923d7f2 100644 (file)
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -326,23 +326,31 @@ skl_program_scaler(struct intel_plane *plane,
        uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
        u16 y_hphase, uv_rgb_hphase;
        u16 y_vphase, uv_rgb_vphase;
+       int hscale, vscale;
+
+       hscale = drm_rect_calc_hscale(&plane_state->base.src,
+                                     &plane_state->base.dst,
+                                     0, INT_MAX);
+       vscale = drm_rect_calc_vscale(&plane_state->base.src,
+                                     &plane_state->base.dst,
+                                     0, INT_MAX);
 
        /* TODO: handle sub-pixel coordinates */
        if (plane_state->base.fb->format->format == DRM_FORMAT_NV12 &&
            !icl_is_hdr_plane(plane)) {
-               y_hphase = skl_scaler_calc_phase(1, false);
-               y_vphase = skl_scaler_calc_phase(1, false);
+               y_hphase = skl_scaler_calc_phase(1, hscale, false);
+               y_vphase = skl_scaler_calc_phase(1, vscale, false);
 
                /* MPEG2 chroma siting convention */
-               uv_rgb_hphase = skl_scaler_calc_phase(2, true);
-               uv_rgb_vphase = skl_scaler_calc_phase(2, false);
+               uv_rgb_hphase = skl_scaler_calc_phase(2, hscale, true);
+               uv_rgb_vphase = skl_scaler_calc_phase(2, vscale, false);
        } else {
                /* not used */
                y_hphase = 0;
                y_vphase = 0;
 
-               uv_rgb_hphase = skl_scaler_calc_phase(1, false);
-               uv_rgb_vphase = skl_scaler_calc_phase(1, false);
+               uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
+               uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
        }
 
        I915_WRITE_FW(SKL_PS_CTRL(pipe, scaler_id),