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 <[email protected]>
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ä <[email protected]> > > > > 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 <[email protected]> > > Fixes: 0a59952b24e2 ("drm/i915: Configure SKL+ scaler initial phase > > correctly") > > Signed-off-by: Ville Syrjälä <[email protected]> > > --- > > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
