From 83234d13f9fda65e6c1a5e5900aa247334b1a621 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 14 Nov 2018 23:07:17 +0200 Subject: [PATCH] drm/i915: Reorganize plane register writes to make them more atomic MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 None of this will have any effect of course unless the vblank evasion fails (which it still does sometimes). Another potential future benefit might be pulling the non-self armings registers outside the vblank evasion since they won't latch until the arming register has been written. This would make the critical section a bit lighter and thus less likely to exceed the deadline. v2: Rebase due to input CSC v3: Swap LINOFF/TILEOFF and KEYMSK/KEYMAX to actually follow the last rule above (Matt) Add a bit more rationale to the commit message (Matt) Cc: Matt Roper Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181114210729.16185-2-ville.syrjala@linux.intel.com Reviewed-by: Matt Roper Reviewed-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 40 +++++++------ drivers/gpu/drm/i915/intel_sprite.c | 108 +++++++++++++++++++++-------------- 2 files changed, 86 insertions(+), 62 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e9f4e22b2a4e..3516b3d04787 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3315,7 +3315,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; @@ -3330,41 +3329,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(DSPLINOFF(i9xx_plane), linear_offset); + I915_WRITE_FW(DSPTILEOFF(i9xx_plane), (y << 16) | 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(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); } @@ -10087,8 +10090,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 @@ -10098,15 +10101,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 abe193815ccc..6cefe5f7a374 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -508,28 +508,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); - if (icl_is_hdr_plane(plane)) { u32 cus_ctl = 0; @@ -551,15 +535,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_KEYMSK(pipe, plane_id), keymsk); + I915_WRITE_FW(PLANE_KEYMAX(pipe, plane_id), keymax); + + 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); } @@ -821,24 +826,29 @@ 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); if (key->flags) { I915_WRITE_FW(SPKEYMINVAL(pipe, plane_id), key->min_value); - I915_WRITE_FW(SPKEYMAXVAL(pipe, plane_id), key->max_value); I915_WRITE_FW(SPKEYMSK(pipe, plane_id), key->channel_mask); + I915_WRITE_FW(SPKEYMAXVAL(pipe, plane_id), key->max_value); } - 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(SPTILEOFF(pipe, plane_id), (y << 16) | x); I915_WRITE_FW(SPLINOFF(pipe, plane_id), linear_offset); + I915_WRITE_FW(SPTILEOFF(pipe, plane_id), (y << 16) | x); - I915_WRITE_FW(SPCONSTALPHA(pipe, plane_id), 0); - - 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); @@ -980,27 +990,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(SPRKEYMAX(pipe), key->max_value); } - 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)) { I915_WRITE_FW(SPROFFSET(pipe), (y << 16) | x); } else { - I915_WRITE_FW(SPRTILEOFF(pipe), (y << 16) | x); I915_WRITE_FW(SPRLINOFF(pipe), linear_offset); + I915_WRITE_FW(SPRTILEOFF(pipe), (y << 16) | x); } - 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); @@ -1018,7 +1033,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); @@ -1148,20 +1163,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(DVSKEYMAX(pipe), key->max_value); } - 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); + I915_WRITE_FW(DVSTILEOFF(pipe), (y << 16) | x); - 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.11.0