On Thu, Feb 10, 2022 at 08:23:59AM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <[email protected]> > > On icl+ all plane registers are armed by PLANE_SURF, so we can > move almost everything over into the update_noarm() hook. > > The PLANE_CTL write has to stay in the icl_update_arm() hook though > as it still exhibits the somewhat annoying self-arming behaviour > when the plane transitioning from disabled to enabled. > > We could either do a full split for skl+ vs. icl+, or we could try > some other kind of split where we'd eg. keep most things in the skl+ > functions and call them from the icl+ functions. I think a full split > is probably the cleaner approach since we've anyway accumulated quite > a bit of icl+ specific things, so that is what I opted to do. > > Some i915_update_info stats for tgl: > before: after: > Updates: 5043 Updates: 5043 > | | > 1us | 1us | > |** |*** > 4us |****** 4us |******** > |********** |*********** > 16us |*********** 16us |********** > |**** |* > 66us | 66us | > | | > 262us | 262us | > | | > 1ms | 1ms | > | | > 4ms | 4ms | > | | > 17ms | 17ms | > | | > Min update: 3494ns Min update: 2983ns > Max update: 49491ns Max update: 39986ns > Average update: 18031ns Average update: 13423ns > Overruns > 100us: 0 Overruns > 100us: 0 > > Signed-off-by: Ville Syrjälä <[email protected]>
Reviewed-by: Stanislav Lisovskiy <[email protected]> > --- > .../drm/i915/display/skl_universal_plane.c | 195 ++++++++++++++---- > 1 file changed, 155 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c > b/drivers/gpu/drm/i915/display/skl_universal_plane.c > index 1223075595ff..5b1b089e045d 100644 > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c > @@ -619,6 +619,25 @@ skl_plane_disable_arm(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > + skl_write_plane_wm(plane, crtc_state); > + > + intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0); > + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0); > + > + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > +} > + > +static void > +icl_plane_disable_arm(struct intel_plane *plane, > + const struct intel_crtc_state *crtc_state) > +{ > + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > + enum plane_id plane_id = plane->id; > + enum pipe pipe = plane->pipe; > + unsigned long irqflags; > + > + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > + > if (icl_is_hdr_plane(dev_priv, plane_id)) > intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0); > > @@ -1065,7 +1084,7 @@ static void icl_plane_csc_load_black(struct intel_plane > *plane) > intel_de_write_fw(i915, PLANE_CSC_POSTOFF(pipe, plane_id, 2), 0); > } > > -static int skl_plane_color_plane(const struct intel_plane_state *plane_state) > +static int icl_plane_color_plane(const struct intel_plane_state *plane_state) > { > /* Program the UV plane on planar master */ > if (plane_state->planar_linked_plane && !plane_state->planar_slave) > @@ -1082,9 +1101,7 @@ skl_plane_update_noarm(struct intel_plane *plane, > struct drm_i915_private *dev_priv = to_i915(plane->base.dev); > enum plane_id plane_id = plane->id; > enum pipe pipe = plane->pipe; > - int color_plane = skl_plane_color_plane(plane_state); > - u32 stride = skl_plane_stride(plane_state, color_plane); > - const struct drm_framebuffer *fb = plane_state->hw.fb; > + u32 stride = skl_plane_stride(plane_state, 0); > int crtc_x = plane_state->uapi.dst.x1; > int crtc_y = plane_state->uapi.dst.y1; > u32 src_w = drm_rect_width(&plane_state->uapi.src) >> 16; > @@ -1099,12 +1116,109 @@ skl_plane_update_noarm(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > + intel_de_write_fw(dev_priv, PLANE_STRIDE(pipe, plane_id), > + PLANE_STRIDE_(stride)); > + intel_de_write_fw(dev_priv, PLANE_POS(pipe, plane_id), > + PLANE_POS_Y(crtc_y) | PLANE_POS_X(crtc_x)); > + intel_de_write_fw(dev_priv, PLANE_SIZE(pipe, plane_id), > + PLANE_HEIGHT(src_h - 1) | PLANE_WIDTH(src_w - 1)); > + > + skl_write_plane_wm(plane, crtc_state); > + > + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > +} > + > +static void > +skl_plane_update_arm(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); > + enum plane_id plane_id = plane->id; > + enum pipe pipe = plane->pipe; > + u32 x = plane_state->view.color_plane[0].x; > + u32 y = plane_state->view.color_plane[0].y; > + u32 plane_ctl, plane_color_ctl = 0; > + unsigned long irqflags; > + > + plane_ctl = plane_state->ctl | > + skl_plane_ctl_crtc(crtc_state); > + > + if (DISPLAY_VER(dev_priv) >= 10) > + plane_color_ctl = plane_state->color_ctl | > + glk_plane_color_ctl_crtc(crtc_state); > + > + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > + > + intel_de_write_fw(dev_priv, PLANE_KEYVAL(pipe, plane_id), > skl_plane_keyval(plane_state)); > + intel_de_write_fw(dev_priv, PLANE_KEYMSK(pipe, plane_id), > skl_plane_keymsk(plane_state)); > + intel_de_write_fw(dev_priv, PLANE_KEYMAX(pipe, plane_id), > skl_plane_keymax(plane_state)); > + > + intel_de_write_fw(dev_priv, PLANE_OFFSET(pipe, plane_id), > + PLANE_OFFSET_Y(y) | PLANE_OFFSET_X(x)); > + > + intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id), > + skl_plane_aux_dist(plane_state, 0)); > + > + intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id), > + PLANE_OFFSET_Y(plane_state->view.color_plane[1].y) | > + PLANE_OFFSET_X(plane_state->view.color_plane[1].x)); > + > + if (DISPLAY_VER(dev_priv) >= 10) > + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), > plane_color_ctl); > + > + /* > + * Enable the scaler before the plane so that we don't > + * get a catastrophic underrun even if the two operations > + * end up happening in two different frames. > + * > + * TODO: split into noarm+arm pair > + */ > + if (plane_state->scaler_id >= 0) > + skl_program_plane_scaler(plane, crtc_state, plane_state); > + > /* > - * FIXME: pxp session invalidation can hit any time even at time of > commit > - * or after the commit, display content will be garbage. > + * The control register self-arms if the plane was previously > + * disabled. Try to make the plane enable atomic by writing > + * the control register just before the surface register. > */ > - if (plane_state->force_black) > - icl_plane_csc_load_black(plane); > + intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), plane_ctl); > + intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), > + skl_plane_surf(plane_state, 0)); > + > + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > +} > + > +static void > +icl_plane_update_noarm(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); > + enum plane_id plane_id = plane->id; > + enum pipe pipe = plane->pipe; > + int color_plane = icl_plane_color_plane(plane_state); > + u32 stride = skl_plane_stride(plane_state, color_plane); > + const struct drm_framebuffer *fb = plane_state->hw.fb; > + int crtc_x = plane_state->uapi.dst.x1; > + int crtc_y = plane_state->uapi.dst.y1; > + int x = plane_state->view.color_plane[color_plane].x; > + int y = plane_state->view.color_plane[color_plane].y; > + int src_w = drm_rect_width(&plane_state->uapi.src) >> 16; > + int src_h = drm_rect_height(&plane_state->uapi.src) >> 16; > + u32 plane_color_ctl; > + unsigned long irqflags; > + > + plane_color_ctl = plane_state->color_ctl | > + glk_plane_color_ctl_crtc(crtc_state); > + > + /* The scaler will handle the output position */ > + if (plane_state->scaler_id >= 0) { > + crtc_x = 0; > + crtc_y = 0; > + } > + > + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > intel_de_write_fw(dev_priv, PLANE_STRIDE(pipe, plane_id), > PLANE_STRIDE_(stride)); > @@ -1113,6 +1227,13 @@ skl_plane_update_noarm(struct intel_plane *plane, > intel_de_write_fw(dev_priv, PLANE_SIZE(pipe, plane_id), > PLANE_HEIGHT(src_h - 1) | PLANE_WIDTH(src_w - 1)); > > + intel_de_write_fw(dev_priv, PLANE_KEYVAL(pipe, plane_id), > skl_plane_keyval(plane_state)); > + intel_de_write_fw(dev_priv, PLANE_KEYMSK(pipe, plane_id), > skl_plane_keymsk(plane_state)); > + intel_de_write_fw(dev_priv, PLANE_KEYMAX(pipe, plane_id), > skl_plane_keymax(plane_state)); > + > + intel_de_write_fw(dev_priv, PLANE_OFFSET(pipe, plane_id), > + PLANE_OFFSET_Y(y) | PLANE_OFFSET_X(x)); > + > if (intel_fb_is_rc_ccs_cc_modifier(fb->modifier)) { > intel_de_write_fw(dev_priv, PLANE_CC_VAL(pipe, plane_id, 0), > lower_32_bits(plane_state->ccval)); > @@ -1120,61 +1241,49 @@ skl_plane_update_noarm(struct intel_plane *plane, > upper_32_bits(plane_state->ccval)); > } > > + intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id), > + skl_plane_aux_dist(plane_state, color_plane)); > + > if (icl_is_hdr_plane(dev_priv, plane_id)) > intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), > plane_state->cus_ctl); > > + intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), > plane_color_ctl); > + > if (fb->format->is_yuv && icl_is_hdr_plane(dev_priv, plane_id)) > icl_program_input_csc(plane, crtc_state, plane_state); > > skl_write_plane_wm(plane, crtc_state); > > + /* > + * FIXME: pxp session invalidation can hit any time even at time of > commit > + * or after the commit, display content will be garbage. > + */ > + if (plane_state->force_black) > + icl_plane_csc_load_black(plane); > + > intel_psr2_program_plane_sel_fetch(plane, crtc_state, plane_state, > color_plane); > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > static void > -skl_plane_update_arm(struct intel_plane *plane, > +icl_plane_update_arm(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); > enum plane_id plane_id = plane->id; > enum pipe pipe = plane->pipe; > - int color_plane = skl_plane_color_plane(plane_state); > - u32 x = plane_state->view.color_plane[color_plane].x; > - u32 y = plane_state->view.color_plane[color_plane].y; > - u32 plane_color_ctl = 0; > - u32 plane_ctl = plane_state->ctl; > + int color_plane = icl_plane_color_plane(plane_state); > + u32 plane_ctl; > unsigned long irqflags; > > - plane_ctl |= skl_plane_ctl_crtc(crtc_state); > - > - if (DISPLAY_VER(dev_priv) >= 10) > - plane_color_ctl = plane_state->color_ctl | > - glk_plane_color_ctl_crtc(crtc_state); > + plane_ctl = plane_state->ctl | > + skl_plane_ctl_crtc(crtc_state); > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > - intel_de_write_fw(dev_priv, PLANE_KEYVAL(pipe, plane_id), > skl_plane_keyval(plane_state)); > - intel_de_write_fw(dev_priv, PLANE_KEYMSK(pipe, plane_id), > skl_plane_keymsk(plane_state)); > - intel_de_write_fw(dev_priv, PLANE_KEYMAX(pipe, plane_id), > skl_plane_keymax(plane_state)); > - > - intel_de_write_fw(dev_priv, PLANE_OFFSET(pipe, plane_id), > - PLANE_OFFSET_Y(y) | PLANE_OFFSET_X(x)); > - > - intel_de_write_fw(dev_priv, PLANE_AUX_DIST(pipe, plane_id), > - skl_plane_aux_dist(plane_state, color_plane)); > - > - if (DISPLAY_VER(dev_priv) < 11) > - intel_de_write_fw(dev_priv, PLANE_AUX_OFFSET(pipe, plane_id), > - > PLANE_OFFSET_Y(plane_state->view.color_plane[1].y) | > - > PLANE_OFFSET_X(plane_state->view.color_plane[1].x)); > - > - if (DISPLAY_VER(dev_priv) >= 10) > - intel_de_write_fw(dev_priv, PLANE_COLOR_CTL(pipe, plane_id), > plane_color_ctl); > - > /* > * Enable the scaler before the plane so that we don't > * get a catastrophic underrun even if the two operations > @@ -2162,9 +2271,15 @@ skl_universal_plane_create(struct drm_i915_private > *dev_priv, > } > > plane->max_stride = skl_plane_max_stride; > - plane->update_noarm = skl_plane_update_noarm; > - plane->update_arm = skl_plane_update_arm; > - plane->disable_arm = skl_plane_disable_arm; > + if (DISPLAY_VER(dev_priv) >= 11) { > + plane->update_noarm = icl_plane_update_noarm; > + plane->update_arm = icl_plane_update_arm; > + plane->disable_arm = icl_plane_disable_arm; > + } else { > + plane->update_noarm = skl_plane_update_noarm; > + plane->update_arm = skl_plane_update_arm; > + plane->disable_arm = skl_plane_disable_arm; > + } > plane->get_hw_state = skl_plane_get_hw_state; > plane->check_plane = skl_plane_check; > > -- > 2.34.1 >
