On Fri, Nov 02, 2018 at 11:47:13AM +0200, Juha-Pekka Heikkila wrote:
> This seems to fix some DRM_FORMAT_RGB565 (up-)scaling IGT tests on on my
> KBL.
>
> Tested-by: Juha-Pekka Heikkila <juhapekka.heikkila@xxxxxxxxx>
Pushed with Maarten's irc r-b and t-b. Thanks for the review and
testing.
>
> On 29.10.2018 20:18, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx>
> > Fixes: 0a59952b24e2 ("drm/i915: Configure SKL+ scaler initial phase correctly")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 45 ++++++++++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_drv.h | 2 +-
> > drivers/gpu/drm/i915/intel_sprite.c | 20 +++++++++----
> > 3 files changed, 57 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index fe045abb6472..33dd2e9751e6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4786,8 +4786,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;
> > @@ -4795,6 +4818,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
> > @@ -5003,13 +5035,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 db24308729b4..86d551a331b1 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1709,7 +1709,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 cfaddc05fea6..fbb916506c77 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -326,27 +326,35 @@ skl_program_scaler(struct drm_i915_private *dev_priv,
> > 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;
> >
> > /* Sizes are 0 based */
> > crtc_w--;
> > crtc_h--;
> >
> > + 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),
> >
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
