On Thu, Sep 20, 2018 at 12:27:10PM +0200, Maarten Lankhorst wrote: > This cleans the code up slightly, and will make other changes easier. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_sprite.c | 90 +++++++++++++++++------------ > 1 file changed, 52 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index d4c8e10fc90b..7d3c7469d271 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -280,13 +280,63 @@ skl_plane_max_stride(struct intel_plane *plane, > return min(8192 * cpp, 32768); > } > > +static void > +skl_program_scaler(struct drm_i915_private *dev_priv, > + struct intel_plane *plane, Could have dug these out from the states rather than passing in explicitly. I prefer minimlism when it comes to function arguments. > + const struct intel_crtc_state *crtc_state, > + const struct intel_plane_state *plane_state) > +{ > + enum plane_id plane_id = plane->id; > + enum pipe pipe = plane->pipe; > + int scaler_id = plane_state->scaler_id; > + const struct intel_scaler *scaler = > + &crtc_state->scaler_state.scalers[scaler_id]; > + int crtc_x = plane_state->base.dst.x1; > + int crtc_y = plane_state->base.dst.y1; > + uint32_t crtc_w = drm_rect_width(&plane_state->base.dst); > + uint32_t crtc_h = drm_rect_height(&plane_state->base.dst); > + u16 y_hphase, uv_rgb_hphase; > + u16 y_vphase, uv_rgb_vphase; > + > + /* Sizes are 0 based */ > + crtc_w--; > + crtc_h--; > + > + /* TODO: handle sub-pixel coordinates */ > + if (plane_state->base.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; > + > + 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)); > +} > + > void > skl_update_plane(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state, > const struct intel_plane_state *plane_state) > { > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > - const struct drm_framebuffer *fb = plane_state->base.fb; > enum plane_id plane_id = plane->id; > enum pipe pipe = plane->pipe; > u32 plane_ctl = plane_state->ctl; > @@ -296,8 +346,6 @@ skl_update_plane(struct intel_plane *plane, > u32 aux_stride = skl_plane_stride(plane_state, 1); > int crtc_x = plane_state->base.dst.x1; > int crtc_y = plane_state->base.dst.y1; > - uint32_t crtc_w = drm_rect_width(&plane_state->base.dst); > - uint32_t crtc_h = drm_rect_height(&plane_state->base.dst); > uint32_t x = plane_state->color_plane[0].x; > uint32_t y = plane_state->color_plane[0].y; > uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16; > @@ -307,8 +355,6 @@ skl_update_plane(struct intel_plane *plane, > /* Sizes are 0 based */ > src_w--; > src_h--; > - crtc_w--; > - crtc_h--; > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > @@ -333,39 +379,7 @@ 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 = > - &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; > - > - 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)); > + skl_program_scaler(dev_priv, plane, crtc_state, plane_state); > > I915_WRITE_FW(PLANE_POS(pipe, plane_id), 0); > } else { > -- > 2.18.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx