On Wed, Apr 22, 2020 at 12:20:06PM -0700, Manasi Navare wrote:
> On Wed, Apr 22, 2020 at 07:19:14PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Make things a bit more abstract by replacing the pch_pfit.pos/size
> > raw register values with a drm_rect. Makes it slighly more convenient
> > to eg. compute the scaling factors.
> >
> > v2: Use drm_rect_init()
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 101 +++++++++++-------
> > .../drm/i915/display/intel_display_types.h | 3 +-
> > drivers/gpu/drm/i915/display/intel_panel.c | 13 ++-
> > 3 files changed, 67 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 96d0768ecf5d..6bb87965801e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6096,10 +6096,8 @@ static int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state)
> > int width, height;
> >
> > if (crtc_state->pch_pfit.enabled) {
> > - u32 pfit_size = crtc_state->pch_pfit.size;
> > -
> > - width = pfit_size >> 16;
> > - height = pfit_size & 0xffff;
> > + width = drm_rect_width(&crtc_state->pch_pfit.dst);
> > + height = drm_rect_height(&crtc_state->pch_pfit.dst);
> > } else {
> > width = adjusted_mode->crtc_hdisplay;
> > height = adjusted_mode->crtc_vdisplay;
> > @@ -6219,11 +6217,20 @@ static void skl_pfit_enable(const struct intel_crtc_state *crtc_state)
> > {
> > struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > - enum pipe pipe = crtc->pipe;
> > const struct intel_crtc_scaler_state *scaler_state =
> > &crtc_state->scaler_state;
> > + struct drm_rect src = {
> > + .x2 = crtc_state->pipe_src_w << 16,
> > + .y2 = crtc_state->pipe_src_h << 16,
>
> Its not clear to me why we left shift by 16 for both src_w and src_h? Where can I find the format of
> how this is stored?
>
> Other than that everything else looks good in terms of replacing with drm_rect()
>
> Manasi
>
<snip>
> > - hscale = (crtc_state->pipe_src_w << 16) / pfit_w;
> > - vscale = (crtc_state->pipe_src_h << 16) / pfit_h;
Same <<16 was here already. skl_scaler_calc_phase() wants
the scaling factor in .16 binary fixed point. Also
drm_rect_calc_{h,v}scale() assumes the src coordinates
to be .16 and returns the result in .16 as well.
> > + hscale = drm_rect_calc_hscale(&src, dst, 0, INT_MAX);
> > + vscale = drm_rect_calc_vscale(&src, dst, 0, INT_MAX);
> >
> > uv_rgb_hphase = skl_scaler_calc_phase(1, hscale, false);
> > uv_rgb_vphase = skl_scaler_calc_phase(1, vscale, false);
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
