| Message ID | 20181107184255.31513-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Wed, Nov 07, 2018 at 08:42:55PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Some observations about the plane registers: > - the control register will self-arm if the plane is not already > enabled, thus we want to write it as close to (or ideally after) > the surface register > - tileoff/linoff/offset/aux_offset are self-arming as well so we want > them close to the surface register as well > - color keying registers we maybe self arming before SKL. Not 100% > sure but we can try to keep them near to the surface register > as well > - chv pipe b csc register are double buffered but self arming so > moving them down a bit > - the rest should be mostly armed by the surface register so we can > safely write them first, and to just for some consistency let's try > to follow keep them in order based on the register offset In general these functions all run under vblank evasion currently, so this won't have a visible effect on atomicity unless we fail the evasion, right? Is the longer-term goal here to eventually pull a bunch of the register writing out and only evade around the actual arming registers? > > v2: Rebase due to input CSC > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 40 +++++---- > drivers/gpu/drm/i915/intel_sprite.c | 120 +++++++++++++++------------ > 2 files changed, 89 insertions(+), 71 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index ae6d58dbf1ed..ac46497cfc52 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3313,7 +3313,6 @@ static void i9xx_update_plane(struct intel_plane *plane, > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > u32 linear_offset; > u32 dspcntr = plane_state->ctl; > - i915_reg_t reg = DSPCNTR(i9xx_plane); > int x = plane_state->color_plane[0].x; > int y = plane_state->color_plane[0].y; > unsigned long irqflags; > @@ -3328,41 +3327,45 @@ static void i9xx_update_plane(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > + I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); > + > if (INTEL_GEN(dev_priv) < 4) { > /* pipesrc and dspsize control the size that is scaled from, > * which should always be the user's requested size. > */ > + I915_WRITE_FW(DSPPOS(i9xx_plane), 0); > I915_WRITE_FW(DSPSIZE(i9xx_plane), > ((crtc_state->pipe_src_h - 1) << 16) | > (crtc_state->pipe_src_w - 1)); > - I915_WRITE_FW(DSPPOS(i9xx_plane), 0); > } else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) { > + I915_WRITE_FW(PRIMPOS(i9xx_plane), 0); > I915_WRITE_FW(PRIMSIZE(i9xx_plane), > ((crtc_state->pipe_src_h - 1) << 16) | > (crtc_state->pipe_src_w - 1)); > - I915_WRITE_FW(PRIMPOS(i9xx_plane), 0); > I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0); > } > > - I915_WRITE_FW(reg, dspcntr); > - > - I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > - I915_WRITE_FW(DSPSURF(i9xx_plane), > - intel_plane_ggtt_offset(plane_state) + > - dspaddr_offset); > I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x); > } else if (INTEL_GEN(dev_priv) >= 4) { > + I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); > + I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset); > + } > + > + /* > + * 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. > + */ > + I915_WRITE_FW(DSPCNTR(i9xx_plane), dspcntr); > + if (INTEL_GEN(dev_priv) >= 4) > I915_WRITE_FW(DSPSURF(i9xx_plane), > intel_plane_ggtt_offset(plane_state) + > dspaddr_offset); > - I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); > - I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset); > - } else { > + else > I915_WRITE_FW(DSPADDR(i9xx_plane), > intel_plane_ggtt_offset(plane_state) + > dspaddr_offset); > - } > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > @@ -10013,8 +10016,8 @@ static void i9xx_update_cursor(struct intel_plane *plane, > * On some platforms writing CURCNTR first will also > * cause CURPOS to be armed by the CURBASE write. > * Without the CURCNTR write the CURPOS write would > - * arm itself. Thus we always start the full update > - * with a CURCNTR write. > + * arm itself. Thus we always update CURCNTR before > + * CURPOS. > * > * On other platforms CURPOS always requires the > * CURBASE write to arm the update. Additonally > @@ -10024,15 +10027,16 @@ static void i9xx_update_cursor(struct intel_plane *plane, > * cursor that doesn't appear to move, or even change > * shape. Thus we always write CURBASE. > * > - * CURCNTR and CUR_FBC_CTL are always > - * armed by the CURBASE write only. > + * The other registers are armed by by the CURBASE write > + * except when the plane is getting enabled at which time > + * the CURCNTR write arms the update. > */ > if (plane->cursor.base != base || > plane->cursor.size != fbc_ctl || > plane->cursor.cntl != cntl) { > - I915_WRITE_FW(CURCNTR(pipe), cntl); > if (HAS_CUR_FBC(dev_priv)) > I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl); > + I915_WRITE_FW(CURCNTR(pipe), cntl); > I915_WRITE_FW(CURPOS(pipe), pos); > I915_WRITE_FW(CURBASE(pipe), base); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index facf7ca8f14f..be6e3c3de1ff 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -500,27 +500,12 @@ skl_program_plane(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > - if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > - I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), > - plane_state->color_ctl); > - > - if (fb->format->is_yuv && icl_is_hdr_plane(plane)) > - icl_program_input_csc_coeff(crtc_state, plane_state); > - > - I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value); > - I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); > - I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk); > - > - I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); > I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); > + I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); > I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w); > I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id), > - (plane_state->color_plane[1].offset - surf_addr) | aux_stride); > - > - if (INTEL_GEN(dev_priv) < 11) > - I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), > - (plane_state->color_plane[1].y << 16) | > - plane_state->color_plane[1].x); > + (plane_state->color_plane[1].offset - surf_addr) | > + aux_stride); > > if (icl_is_hdr_plane(plane)) { > u32 cus_ctl = 0; > @@ -543,15 +528,36 @@ skl_program_plane(struct intel_plane *plane, > I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), cus_ctl); > } > > - if (!slave && plane_state->scaler_id >= 0) > - skl_program_scaler(plane, crtc_state, plane_state); > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > + I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), > + plane_state->color_ctl); > > - I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); > + if (fb->format->is_yuv && icl_is_hdr_plane(plane)) > + icl_program_input_csc_coeff(crtc_state, plane_state); > + > + I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value); > + I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); > + I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk); You mentioned above that you wanted to follow register offset ordering where possible, so I believe MSK should come before MAX here. Same comment for the vlv and g4x functions farther down. > + > + I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); > + > + if (INTEL_GEN(dev_priv) < 11) > + I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), > + (plane_state->color_plane[1].y << 16) | > + plane_state->color_plane[1].x); > > + /* > + * 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. > + */ > I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl); > I915_WRITE_FW(PLANE_SURF(pipe, plane_id), > intel_plane_ggtt_offset(plane_state) + surf_addr); > > + if (!slave && plane_state->scaler_id >= 0) > + skl_program_scaler(plane, crtc_state, plane_state); > + > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > } > > @@ -789,7 +795,6 @@ vlv_update_plane(struct intel_plane *plane, > 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 pipe pipe = plane->pipe; > enum plane_id plane_id = plane->id; > u32 sprctl = plane_state->ctl; > @@ -814,6 +819,12 @@ vlv_update_plane(struct intel_plane *plane, > > vlv_update_clrc(plane_state); > > + I915_WRITE_FW(SPSTRIDE(pipe, plane_id), > + plane_state->color_plane[0].stride); > + I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); > + I915_WRITE_FW(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w); > + I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0); > + > if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) > chv_update_csc(plane_state); > > @@ -822,18 +833,15 @@ vlv_update_plane(struct intel_plane *plane, > I915_WRITE_FW(SPKEYMAXVAL(pipe, plane_id), key->max_value); > I915_WRITE_FW(SPKEYMSK(pipe, plane_id), key->channel_mask); > } > - I915_WRITE_FW(SPSTRIDE(pipe, plane_id), > - plane_state->color_plane[0].stride); > - I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); > - > - if (fb->modifier == I915_FORMAT_MOD_X_TILED) > - I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x); > - else > - I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset); > > - I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0); > + I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x); > + I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset); I think you already indicated you were going to pull the "ignore tiling and just write both offset registers" out into a separate patch, but these are another case where you might want to flip the order to match register offset (linear < tiled). Matt > > - I915_WRITE_FW(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w); > + /* > + * 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. > + */ > I915_WRITE_FW(SPCNTR(pipe, plane_id), sprctl); > I915_WRITE_FW(SPSURF(pipe, plane_id), > intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > @@ -947,7 +955,6 @@ ivb_update_plane(struct intel_plane *plane, > 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 pipe pipe = plane->pipe; > u32 sprctl = plane_state->ctl, sprscale = 0; > u32 sprsurf_offset = plane_state->color_plane[0].offset; > @@ -976,27 +983,32 @@ ivb_update_plane(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > + I915_WRITE_FW(SPRSTRIDE(pipe), plane_state->color_plane[0].stride); > + I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x); > + I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w); > + if (IS_IVYBRIDGE(dev_priv)) > + I915_WRITE_FW(SPRSCALE(pipe), sprscale); > + > if (key->flags) { > I915_WRITE_FW(SPRKEYVAL(pipe), key->min_value); > I915_WRITE_FW(SPRKEYMAX(pipe), key->max_value); > I915_WRITE_FW(SPRKEYMSK(pipe), key->channel_mask); > } > > - I915_WRITE_FW(SPRSTRIDE(pipe), plane_state->color_plane[0].stride); > - I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x); > - > /* HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET > * register */ > - if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > I915_WRITE_FW(SPROFFSET(pipe), (y << 16) | x); > - else if (fb->modifier == I915_FORMAT_MOD_X_TILED) > + } else { > I915_WRITE_FW(SPRTILEOFF(pipe), (y << 16) | x); > - else > I915_WRITE_FW(SPRLINOFF(pipe), linear_offset); > + } > > - I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w); > - if (IS_IVYBRIDGE(dev_priv)) > - I915_WRITE_FW(SPRSCALE(pipe), sprscale); > + /* > + * 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. > + */ > I915_WRITE_FW(SPRCTL(pipe), sprctl); > I915_WRITE_FW(SPRSURF(pipe), > intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > @@ -1014,7 +1026,7 @@ ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > I915_WRITE_FW(SPRCTL(pipe), 0); > - /* Can't leave the scaler enabled... */ > + /* Disable the scaler */ > if (IS_IVYBRIDGE(dev_priv)) > I915_WRITE_FW(SPRSCALE(pipe), 0); > I915_WRITE_FW(SPRSURF(pipe), 0); > @@ -1116,7 +1128,6 @@ g4x_update_plane(struct intel_plane *plane, > 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 pipe pipe = plane->pipe; > u32 dvscntr = plane_state->ctl, dvsscale = 0; > u32 dvssurf_offset = plane_state->color_plane[0].offset; > @@ -1145,22 +1156,25 @@ g4x_update_plane(struct intel_plane *plane, > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > + I915_WRITE_FW(DVSSTRIDE(pipe), plane_state->color_plane[0].stride); > + I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x); > + I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); > + I915_WRITE_FW(DVSSCALE(pipe), dvsscale); > + > if (key->flags) { > I915_WRITE_FW(DVSKEYVAL(pipe), key->min_value); > I915_WRITE_FW(DVSKEYMAX(pipe), key->max_value); > I915_WRITE_FW(DVSKEYMSK(pipe), key->channel_mask); > } > > - I915_WRITE_FW(DVSSTRIDE(pipe), plane_state->color_plane[0].stride); > - I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x); > + I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x); > + I915_WRITE_FW(DVSLINOFF(pipe), linear_offset); > > - if (fb->modifier == I915_FORMAT_MOD_X_TILED) > - I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x); > - else > - I915_WRITE_FW(DVSLINOFF(pipe), linear_offset); > - > - I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); > - I915_WRITE_FW(DVSSCALE(pipe), dvsscale); > + /* > + * 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. > + */ > I915_WRITE_FW(DVSCNTR(pipe), dvscntr); > I915_WRITE_FW(DVSSURF(pipe), > intel_plane_ggtt_offset(plane_state) + dvssurf_offset); > -- > 2.18.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Nov 08, 2018 at 11:30:07AM -0800, Matt Roper wrote: > On Wed, Nov 07, 2018 at 08:42:55PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Some observations about the plane registers: > > - the control register will self-arm if the plane is not already > > enabled, thus we want to write it as close to (or ideally after) > > the surface register > > - tileoff/linoff/offset/aux_offset are self-arming as well so we want > > them close to the surface register as well > > - color keying registers we maybe self arming before SKL. Not 100% > > sure but we can try to keep them near to the surface register > > as well > > - chv pipe b csc register are double buffered but self arming so > > moving them down a bit > > - the rest should be mostly armed by the surface register so we can > > safely write them first, and to just for some consistency let's try > > to follow keep them in order based on the register offset > > In general these functions all run under vblank evasion currently, so > this won't have a visible effect on atomicity unless we fail the > evasion, right? Yes. > Is the longer-term goal here to eventually pull a bunch > of the register writing out and only evade around the actual arming > registers? That has been on my mind off and on. But I haven't actually measured how much we'd gain. So not sure if it's worth the hassle currently. We may need to think much harder on this when our mmio becomes more expensive... > > > > > v2: Rebase due to input CSC > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 40 +++++---- > > drivers/gpu/drm/i915/intel_sprite.c | 120 +++++++++++++++------------ > > 2 files changed, 89 insertions(+), 71 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index ae6d58dbf1ed..ac46497cfc52 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3313,7 +3313,6 @@ static void i9xx_update_plane(struct intel_plane *plane, > > enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; > > u32 linear_offset; > > u32 dspcntr = plane_state->ctl; > > - i915_reg_t reg = DSPCNTR(i9xx_plane); > > int x = plane_state->color_plane[0].x; > > int y = plane_state->color_plane[0].y; > > unsigned long irqflags; > > @@ -3328,41 +3327,45 @@ static void i9xx_update_plane(struct intel_plane *plane, > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > + I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); > > + > > if (INTEL_GEN(dev_priv) < 4) { > > /* pipesrc and dspsize control the size that is scaled from, > > * which should always be the user's requested size. > > */ > > + I915_WRITE_FW(DSPPOS(i9xx_plane), 0); > > I915_WRITE_FW(DSPSIZE(i9xx_plane), > > ((crtc_state->pipe_src_h - 1) << 16) | > > (crtc_state->pipe_src_w - 1)); > > - I915_WRITE_FW(DSPPOS(i9xx_plane), 0); > > } else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) { > > + I915_WRITE_FW(PRIMPOS(i9xx_plane), 0); > > I915_WRITE_FW(PRIMSIZE(i9xx_plane), > > ((crtc_state->pipe_src_h - 1) << 16) | > > (crtc_state->pipe_src_w - 1)); > > - I915_WRITE_FW(PRIMPOS(i9xx_plane), 0); > > I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0); > > } > > > > - I915_WRITE_FW(reg, dspcntr); > > - > > - I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > - I915_WRITE_FW(DSPSURF(i9xx_plane), > > - intel_plane_ggtt_offset(plane_state) + > > - dspaddr_offset); > > I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x); > > } else if (INTEL_GEN(dev_priv) >= 4) { > > + I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); > > + I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset); > > + } > > + > > + /* > > + * 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. > > + */ > > + I915_WRITE_FW(DSPCNTR(i9xx_plane), dspcntr); > > + if (INTEL_GEN(dev_priv) >= 4) > > I915_WRITE_FW(DSPSURF(i9xx_plane), > > intel_plane_ggtt_offset(plane_state) + > > dspaddr_offset); > > - I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); > > - I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset); > > - } else { > > + else > > I915_WRITE_FW(DSPADDR(i9xx_plane), > > intel_plane_ggtt_offset(plane_state) + > > dspaddr_offset); > > - } > > > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > > } > > @@ -10013,8 +10016,8 @@ static void i9xx_update_cursor(struct intel_plane *plane, > > * On some platforms writing CURCNTR first will also > > * cause CURPOS to be armed by the CURBASE write. > > * Without the CURCNTR write the CURPOS write would > > - * arm itself. Thus we always start the full update > > - * with a CURCNTR write. > > + * arm itself. Thus we always update CURCNTR before > > + * CURPOS. > > * > > * On other platforms CURPOS always requires the > > * CURBASE write to arm the update. Additonally > > @@ -10024,15 +10027,16 @@ static void i9xx_update_cursor(struct intel_plane *plane, > > * cursor that doesn't appear to move, or even change > > * shape. Thus we always write CURBASE. > > * > > - * CURCNTR and CUR_FBC_CTL are always > > - * armed by the CURBASE write only. > > + * The other registers are armed by by the CURBASE write > > + * except when the plane is getting enabled at which time > > + * the CURCNTR write arms the update. > > */ > > if (plane->cursor.base != base || > > plane->cursor.size != fbc_ctl || > > plane->cursor.cntl != cntl) { > > - I915_WRITE_FW(CURCNTR(pipe), cntl); > > if (HAS_CUR_FBC(dev_priv)) > > I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl); > > + I915_WRITE_FW(CURCNTR(pipe), cntl); > > I915_WRITE_FW(CURPOS(pipe), pos); > > I915_WRITE_FW(CURBASE(pipe), base); > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index facf7ca8f14f..be6e3c3de1ff 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -500,27 +500,12 @@ skl_program_plane(struct intel_plane *plane, > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > - if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > > - I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), > > - plane_state->color_ctl); > > - > > - if (fb->format->is_yuv && icl_is_hdr_plane(plane)) > > - icl_program_input_csc_coeff(crtc_state, plane_state); > > - > > - I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value); > > - I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); > > - I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk); > > - > > - I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); > > I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); > > + I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); > > I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w); > > I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id), > > - (plane_state->color_plane[1].offset - surf_addr) | aux_stride); > > - > > - if (INTEL_GEN(dev_priv) < 11) > > - I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), > > - (plane_state->color_plane[1].y << 16) | > > - plane_state->color_plane[1].x); > > + (plane_state->color_plane[1].offset - surf_addr) | > > + aux_stride); > > > > if (icl_is_hdr_plane(plane)) { > > u32 cus_ctl = 0; > > @@ -543,15 +528,36 @@ skl_program_plane(struct intel_plane *plane, > > I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), cus_ctl); > > } > > > > - if (!slave && plane_state->scaler_id >= 0) > > - skl_program_scaler(plane, crtc_state, plane_state); > > + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) > > + I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), > > + plane_state->color_ctl); > > > > - I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); > > + if (fb->format->is_yuv && icl_is_hdr_plane(plane)) > > + icl_program_input_csc_coeff(crtc_state, plane_state); > > + > > + I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value); > > + I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); > > + I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk); > > You mentioned above that you wanted to follow register offset ordering > where possible, so I believe MSK should come before MAX here. Same > comment for the vlv and g4x functions farther down. > > > + > > + I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); > > + > > + if (INTEL_GEN(dev_priv) < 11) > > + I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), > > + (plane_state->color_plane[1].y << 16) | > > + plane_state->color_plane[1].x); > > > > + /* > > + * 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. > > + */ > > I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl); > > I915_WRITE_FW(PLANE_SURF(pipe, plane_id), > > intel_plane_ggtt_offset(plane_state) + surf_addr); > > > > + if (!slave && plane_state->scaler_id >= 0) > > + skl_program_scaler(plane, crtc_state, plane_state); > > + > > spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); > > } > > > > @@ -789,7 +795,6 @@ vlv_update_plane(struct intel_plane *plane, > > 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 pipe pipe = plane->pipe; > > enum plane_id plane_id = plane->id; > > u32 sprctl = plane_state->ctl; > > @@ -814,6 +819,12 @@ vlv_update_plane(struct intel_plane *plane, > > > > vlv_update_clrc(plane_state); > > > > + I915_WRITE_FW(SPSTRIDE(pipe, plane_id), > > + plane_state->color_plane[0].stride); > > + I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); > > + I915_WRITE_FW(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w); > > + I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0); > > + > > if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) > > chv_update_csc(plane_state); > > > > @@ -822,18 +833,15 @@ vlv_update_plane(struct intel_plane *plane, > > I915_WRITE_FW(SPKEYMAXVAL(pipe, plane_id), key->max_value); > > I915_WRITE_FW(SPKEYMSK(pipe, plane_id), key->channel_mask); > > } > > - I915_WRITE_FW(SPSTRIDE(pipe, plane_id), > > - plane_state->color_plane[0].stride); > > - I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); > > - > > - if (fb->modifier == I915_FORMAT_MOD_X_TILED) > > - I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x); > > - else > > - I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset); > > > > - I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0); > > + I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x); > > + I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset); > > I think you already indicated you were going to pull the "ignore tiling > and just write both offset registers" out into a separate patch, but > these are another case where you might want to flip the order to match > register offset (linear < tiled). Sure. It looks like my ocd wasn't quite strong enough to follow my own rule 100% :) > > > Matt > > > > > - I915_WRITE_FW(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w); > > + /* > > + * 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. > > + */ > > I915_WRITE_FW(SPCNTR(pipe, plane_id), sprctl); > > I915_WRITE_FW(SPSURF(pipe, plane_id), > > intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > > @@ -947,7 +955,6 @@ ivb_update_plane(struct intel_plane *plane, > > 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 pipe pipe = plane->pipe; > > u32 sprctl = plane_state->ctl, sprscale = 0; > > u32 sprsurf_offset = plane_state->color_plane[0].offset; > > @@ -976,27 +983,32 @@ ivb_update_plane(struct intel_plane *plane, > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > + I915_WRITE_FW(SPRSTRIDE(pipe), plane_state->color_plane[0].stride); > > + I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x); > > + I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w); > > + if (IS_IVYBRIDGE(dev_priv)) > > + I915_WRITE_FW(SPRSCALE(pipe), sprscale); > > + > > if (key->flags) { > > I915_WRITE_FW(SPRKEYVAL(pipe), key->min_value); > > I915_WRITE_FW(SPRKEYMAX(pipe), key->max_value); > > I915_WRITE_FW(SPRKEYMSK(pipe), key->channel_mask); > > } > > > > - I915_WRITE_FW(SPRSTRIDE(pipe), plane_state->color_plane[0].stride); > > - I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x); > > - > > /* HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET > > * register */ > > - if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > > + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > I915_WRITE_FW(SPROFFSET(pipe), (y << 16) | x); > > - else if (fb->modifier == I915_FORMAT_MOD_X_TILED) > > + } else { > > I915_WRITE_FW(SPRTILEOFF(pipe), (y << 16) | x); > > - else > > I915_WRITE_FW(SPRLINOFF(pipe), linear_offset); > > + } > > > > - I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w); > > - if (IS_IVYBRIDGE(dev_priv)) > > - I915_WRITE_FW(SPRSCALE(pipe), sprscale); > > + /* > > + * 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. > > + */ > > I915_WRITE_FW(SPRCTL(pipe), sprctl); > > I915_WRITE_FW(SPRSURF(pipe), > > intel_plane_ggtt_offset(plane_state) + sprsurf_offset); > > @@ -1014,7 +1026,7 @@ ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > I915_WRITE_FW(SPRCTL(pipe), 0); > > - /* Can't leave the scaler enabled... */ > > + /* Disable the scaler */ > > if (IS_IVYBRIDGE(dev_priv)) > > I915_WRITE_FW(SPRSCALE(pipe), 0); > > I915_WRITE_FW(SPRSURF(pipe), 0); > > @@ -1116,7 +1128,6 @@ g4x_update_plane(struct intel_plane *plane, > > 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 pipe pipe = plane->pipe; > > u32 dvscntr = plane_state->ctl, dvsscale = 0; > > u32 dvssurf_offset = plane_state->color_plane[0].offset; > > @@ -1145,22 +1156,25 @@ g4x_update_plane(struct intel_plane *plane, > > > > spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); > > > > + I915_WRITE_FW(DVSSTRIDE(pipe), plane_state->color_plane[0].stride); > > + I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x); > > + I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); > > + I915_WRITE_FW(DVSSCALE(pipe), dvsscale); > > + > > if (key->flags) { > > I915_WRITE_FW(DVSKEYVAL(pipe), key->min_value); > > I915_WRITE_FW(DVSKEYMAX(pipe), key->max_value); > > I915_WRITE_FW(DVSKEYMSK(pipe), key->channel_mask); > > } > > > > - I915_WRITE_FW(DVSSTRIDE(pipe), plane_state->color_plane[0].stride); > > - I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x); > > + I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x); > > + I915_WRITE_FW(DVSLINOFF(pipe), linear_offset); > > > > - if (fb->modifier == I915_FORMAT_MOD_X_TILED) > > - I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x); > > - else > > - I915_WRITE_FW(DVSLINOFF(pipe), linear_offset); > > - > > - I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); > > - I915_WRITE_FW(DVSSCALE(pipe), dvsscale); > > + /* > > + * 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. > > + */ > > I915_WRITE_FW(DVSCNTR(pipe), dvscntr); > > I915_WRITE_FW(DVSSURF(pipe), > > intel_plane_ggtt_offset(plane_state) + dvssurf_offset); > > -- > > 2.18.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ae6d58dbf1ed..ac46497cfc52 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3313,7 +3313,6 @@ static void i9xx_update_plane(struct intel_plane *plane, enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; u32 linear_offset; u32 dspcntr = plane_state->ctl; - i915_reg_t reg = DSPCNTR(i9xx_plane); int x = plane_state->color_plane[0].x; int y = plane_state->color_plane[0].y; unsigned long irqflags; @@ -3328,41 +3327,45 @@ static void i9xx_update_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); + I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); + if (INTEL_GEN(dev_priv) < 4) { /* pipesrc and dspsize control the size that is scaled from, * which should always be the user's requested size. */ + I915_WRITE_FW(DSPPOS(i9xx_plane), 0); I915_WRITE_FW(DSPSIZE(i9xx_plane), ((crtc_state->pipe_src_h - 1) << 16) | (crtc_state->pipe_src_w - 1)); - I915_WRITE_FW(DSPPOS(i9xx_plane), 0); } else if (IS_CHERRYVIEW(dev_priv) && i9xx_plane == PLANE_B) { + I915_WRITE_FW(PRIMPOS(i9xx_plane), 0); I915_WRITE_FW(PRIMSIZE(i9xx_plane), ((crtc_state->pipe_src_h - 1) << 16) | (crtc_state->pipe_src_w - 1)); - I915_WRITE_FW(PRIMPOS(i9xx_plane), 0); I915_WRITE_FW(PRIMCNSTALPHA(i9xx_plane), 0); } - I915_WRITE_FW(reg, dspcntr); - - I915_WRITE_FW(DSPSTRIDE(i9xx_plane), plane_state->color_plane[0].stride); if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { - I915_WRITE_FW(DSPSURF(i9xx_plane), - intel_plane_ggtt_offset(plane_state) + - dspaddr_offset); I915_WRITE_FW(DSPOFFSET(i9xx_plane), (y << 16) | x); } else if (INTEL_GEN(dev_priv) >= 4) { + I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); + I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset); + } + + /* + * 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. + */ + I915_WRITE_FW(DSPCNTR(i9xx_plane), dspcntr); + if (INTEL_GEN(dev_priv) >= 4) I915_WRITE_FW(DSPSURF(i9xx_plane), intel_plane_ggtt_offset(plane_state) + dspaddr_offset); - I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | x); - I915_WRITE_FW(DSPLINOFF(i9xx_plane), linear_offset); - } else { + else I915_WRITE_FW(DSPADDR(i9xx_plane), intel_plane_ggtt_offset(plane_state) + dspaddr_offset); - } spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); } @@ -10013,8 +10016,8 @@ static void i9xx_update_cursor(struct intel_plane *plane, * On some platforms writing CURCNTR first will also * cause CURPOS to be armed by the CURBASE write. * Without the CURCNTR write the CURPOS write would - * arm itself. Thus we always start the full update - * with a CURCNTR write. + * arm itself. Thus we always update CURCNTR before + * CURPOS. * * On other platforms CURPOS always requires the * CURBASE write to arm the update. Additonally @@ -10024,15 +10027,16 @@ static void i9xx_update_cursor(struct intel_plane *plane, * cursor that doesn't appear to move, or even change * shape. Thus we always write CURBASE. * - * CURCNTR and CUR_FBC_CTL are always - * armed by the CURBASE write only. + * The other registers are armed by by the CURBASE write + * except when the plane is getting enabled at which time + * the CURCNTR write arms the update. */ if (plane->cursor.base != base || plane->cursor.size != fbc_ctl || plane->cursor.cntl != cntl) { - I915_WRITE_FW(CURCNTR(pipe), cntl); if (HAS_CUR_FBC(dev_priv)) I915_WRITE_FW(CUR_FBC_CTL(pipe), fbc_ctl); + I915_WRITE_FW(CURCNTR(pipe), cntl); I915_WRITE_FW(CURPOS(pipe), pos); I915_WRITE_FW(CURBASE(pipe), base); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index facf7ca8f14f..be6e3c3de1ff 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -500,27 +500,12 @@ skl_program_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) - I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), - plane_state->color_ctl); - - if (fb->format->is_yuv && icl_is_hdr_plane(plane)) - icl_program_input_csc_coeff(crtc_state, plane_state); - - I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value); - I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); - I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk); - - I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride); + I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w); I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id), - (plane_state->color_plane[1].offset - surf_addr) | aux_stride); - - if (INTEL_GEN(dev_priv) < 11) - I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), - (plane_state->color_plane[1].y << 16) | - plane_state->color_plane[1].x); + (plane_state->color_plane[1].offset - surf_addr) | + aux_stride); if (icl_is_hdr_plane(plane)) { u32 cus_ctl = 0; @@ -543,15 +528,36 @@ skl_program_plane(struct intel_plane *plane, I915_WRITE_FW(PLANE_CUS_CTL(pipe, plane_id), cus_ctl); } - if (!slave && plane_state->scaler_id >= 0) - skl_program_scaler(plane, crtc_state, plane_state); + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) + I915_WRITE_FW(PLANE_COLOR_CTL(pipe, plane_id), + plane_state->color_ctl); - I915_WRITE_FW(PLANE_POS(pipe, plane_id), (crtc_y << 16) | crtc_x); + if (fb->format->is_yuv && icl_is_hdr_plane(plane)) + icl_program_input_csc_coeff(crtc_state, plane_state); + + I915_WRITE_FW(PLANE_KEYVAL(pipe, plane_id), key->min_value); + I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); + I915_WRITE_FW(PLANE_KEYMSK(pipe, plane_id), keymsk); + + I915_WRITE_FW(PLANE_OFFSET(pipe, plane_id), (y << 16) | x); + + if (INTEL_GEN(dev_priv) < 11) + I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id), + (plane_state->color_plane[1].y << 16) | + plane_state->color_plane[1].x); + /* + * 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. + */ I915_WRITE_FW(PLANE_CTL(pipe, plane_id), plane_ctl); I915_WRITE_FW(PLANE_SURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + surf_addr); + if (!slave && plane_state->scaler_id >= 0) + skl_program_scaler(plane, crtc_state, plane_state); + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); } @@ -789,7 +795,6 @@ vlv_update_plane(struct intel_plane *plane, 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 pipe pipe = plane->pipe; enum plane_id plane_id = plane->id; u32 sprctl = plane_state->ctl; @@ -814,6 +819,12 @@ vlv_update_plane(struct intel_plane *plane, vlv_update_clrc(plane_state); + I915_WRITE_FW(SPSTRIDE(pipe, plane_id), + plane_state->color_plane[0].stride); + I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); + I915_WRITE_FW(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w); + I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0); + if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B) chv_update_csc(plane_state); @@ -822,18 +833,15 @@ vlv_update_plane(struct intel_plane *plane, I915_WRITE_FW(SPKEYMAXVAL(pipe, plane_id), key->max_value); I915_WRITE_FW(SPKEYMSK(pipe, plane_id), key->channel_mask); } - I915_WRITE_FW(SPSTRIDE(pipe, plane_id), - plane_state->color_plane[0].stride); - I915_WRITE_FW(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x); - - if (fb->modifier == I915_FORMAT_MOD_X_TILED) - I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x); - else - I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset); - I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0); + I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x); + I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset); - I915_WRITE_FW(SPSIZE(pipe, plane_id), (crtc_h << 16) | crtc_w); + /* + * 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. + */ I915_WRITE_FW(SPCNTR(pipe, plane_id), sprctl); I915_WRITE_FW(SPSURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + sprsurf_offset); @@ -947,7 +955,6 @@ ivb_update_plane(struct intel_plane *plane, 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 pipe pipe = plane->pipe; u32 sprctl = plane_state->ctl, sprscale = 0; u32 sprsurf_offset = plane_state->color_plane[0].offset; @@ -976,27 +983,32 @@ ivb_update_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); + I915_WRITE_FW(SPRSTRIDE(pipe), plane_state->color_plane[0].stride); + I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x); + I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w); + if (IS_IVYBRIDGE(dev_priv)) + I915_WRITE_FW(SPRSCALE(pipe), sprscale); + if (key->flags) { I915_WRITE_FW(SPRKEYVAL(pipe), key->min_value); I915_WRITE_FW(SPRKEYMAX(pipe), key->max_value); I915_WRITE_FW(SPRKEYMSK(pipe), key->channel_mask); } - I915_WRITE_FW(SPRSTRIDE(pipe), plane_state->color_plane[0].stride); - I915_WRITE_FW(SPRPOS(pipe), (crtc_y << 16) | crtc_x); - /* HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET * register */ - if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { I915_WRITE_FW(SPROFFSET(pipe), (y << 16) | x); - else if (fb->modifier == I915_FORMAT_MOD_X_TILED) + } else { I915_WRITE_FW(SPRTILEOFF(pipe), (y << 16) | x); - else I915_WRITE_FW(SPRLINOFF(pipe), linear_offset); + } - I915_WRITE_FW(SPRSIZE(pipe), (crtc_h << 16) | crtc_w); - if (IS_IVYBRIDGE(dev_priv)) - I915_WRITE_FW(SPRSCALE(pipe), sprscale); + /* + * 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. + */ I915_WRITE_FW(SPRCTL(pipe), sprctl); I915_WRITE_FW(SPRSURF(pipe), intel_plane_ggtt_offset(plane_state) + sprsurf_offset); @@ -1014,7 +1026,7 @@ ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); I915_WRITE_FW(SPRCTL(pipe), 0); - /* Can't leave the scaler enabled... */ + /* Disable the scaler */ if (IS_IVYBRIDGE(dev_priv)) I915_WRITE_FW(SPRSCALE(pipe), 0); I915_WRITE_FW(SPRSURF(pipe), 0); @@ -1116,7 +1128,6 @@ g4x_update_plane(struct intel_plane *plane, 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 pipe pipe = plane->pipe; u32 dvscntr = plane_state->ctl, dvsscale = 0; u32 dvssurf_offset = plane_state->color_plane[0].offset; @@ -1145,22 +1156,25 @@ g4x_update_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); + I915_WRITE_FW(DVSSTRIDE(pipe), plane_state->color_plane[0].stride); + I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x); + I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); + I915_WRITE_FW(DVSSCALE(pipe), dvsscale); + if (key->flags) { I915_WRITE_FW(DVSKEYVAL(pipe), key->min_value); I915_WRITE_FW(DVSKEYMAX(pipe), key->max_value); I915_WRITE_FW(DVSKEYMSK(pipe), key->channel_mask); } - I915_WRITE_FW(DVSSTRIDE(pipe), plane_state->color_plane[0].stride); - I915_WRITE_FW(DVSPOS(pipe), (crtc_y << 16) | crtc_x); + I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x); + I915_WRITE_FW(DVSLINOFF(pipe), linear_offset); - if (fb->modifier == I915_FORMAT_MOD_X_TILED) - I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x); - else - I915_WRITE_FW(DVSLINOFF(pipe), linear_offset); - - I915_WRITE_FW(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); - I915_WRITE_FW(DVSSCALE(pipe), dvsscale); + /* + * 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. + */ I915_WRITE_FW(DVSCNTR(pipe), dvscntr); I915_WRITE_FW(DVSSURF(pipe), intel_plane_ggtt_offset(plane_state) + dvssurf_offset);