From 83234d13f9fda65e6c1a5e5900aa247334b1a621 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 14 Nov 2018 23:07:17 +0200 Subject: drm/i915: Reorganize plane register writes to make them more atomic 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_sprite.c | 108 +++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 44 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_sprite.c') 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); -- cgit v1.2.3-55-g7522 From 019575a58c84b1cd534d1dd1cad36592995f8f6b Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 14 Nov 2018 23:07:18 +0200 Subject: drm/i915: Move single buffered plane register writes to the end The plane color correction registers are single buffered. So ideally we would write them at the start of vblank just after the double buffered plane registers have been latched. Since we have no convenient way to do that for now let's at least move the single buffered register writes to happen after the double buffered registers have been written. Reviewed-by: Rodrigo Vivi Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181114210729.16185-3-ville.syrjala@linux.intel.com Reviewed-by: Matt Roper Reviewed-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_sprite.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_sprite.c') diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 6cefe5f7a374..0548b996693f 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -824,8 +824,6 @@ vlv_update_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); - 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); @@ -853,6 +851,8 @@ vlv_update_plane(struct intel_plane *plane, I915_WRITE_FW(SPSURF(pipe, plane_id), intel_plane_ggtt_offset(plane_state) + sprsurf_offset); + vlv_update_clrc(plane_state); + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); } -- cgit v1.2.3-55-g7522 From 0dd14be30d4c8401be9cd129df3b3ee238ea6f78 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 14 Nov 2018 23:07:20 +0200 Subject: drm/i915: Pass the new crtc_state to ->disable_plane() We're going to need access to the new crtc state in ->disable_plane() for SKL+ wm/ddb programming and pre-skl pipe gamma/csc control. Pass the crtc state down. We'll also try to make intel_crtc_disable_planes() do the right thing as much as it's possible. The fact that we don't have a separate crtc state for the disabled state when we're going to re-enable the crtc later means we might end up poking at a few extra planes in there. But that's harmless. I suppose one might argue that we wouldn't have to care about proper ddb/wm/csc/gamma if the pipe is going to permanently disable anyway, but the state checker probably cares so we should try our best to make sure everything is programmed correctly even in that case. v2: Fix the commit message a bit (Matt) Cc: Matt Roper Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181114210729.16185-5-ville.syrjala@linux.intel.com Reviewed-by: Matt Roper Reviewed-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_atomic_plane.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++------------ drivers/gpu/drm/i915/intel_display.h | 8 +++++++ drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_sprite.c | 12 ++++++---- 5 files changed, 42 insertions(+), 21 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_sprite.c') diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 026a11511941..ad3d135bdc15 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -212,7 +212,7 @@ void intel_update_planes_on_crtc(struct intel_atomic_state *old_state, } else { trace_intel_disable_plane(&plane->base, crtc); - plane->disable_plane(plane, crtc); + plane->disable_plane(plane, new_crtc_state); } } } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4469b4511c92..d088ffd9f542 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2767,7 +2767,7 @@ static void intel_plane_disable_noatomic(struct intel_crtc *crtc, intel_pre_disable_primary_noatomic(&crtc->base); trace_intel_disable_plane(&plane->base, crtc); - plane->disable_plane(plane, crtc); + plane->disable_plane(plane, crtc_state); } static void @@ -3373,7 +3373,7 @@ static void i9xx_update_plane(struct intel_plane *plane, } static void i9xx_disable_plane(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum i9xx_plane_id i9xx_plane = plane->i9xx_plane; @@ -5406,23 +5406,32 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, intel_update_watermarks(crtc); } -static void intel_crtc_disable_planes(struct intel_crtc *crtc, unsigned plane_mask) +static void intel_crtc_disable_planes(struct intel_atomic_state *state, + struct intel_crtc *crtc) { - struct drm_device *dev = crtc->base.dev; + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + const struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + unsigned int update_mask = new_crtc_state->update_planes; + const struct intel_plane_state *old_plane_state; struct intel_plane *plane; unsigned fb_bits = 0; + int i; intel_crtc_dpms_overlay_disable(crtc); - for_each_intel_plane_on_crtc(dev, crtc, plane) { - if (plane_mask & BIT(plane->id)) { - plane->disable_plane(plane, crtc); + for_each_old_intel_plane_in_state(state, plane, old_plane_state, i) { + if (crtc->pipe != plane->pipe || + !(update_mask & BIT(plane->id))) + continue; + + plane->disable_plane(plane, new_crtc_state); + if (old_plane_state->base.visible) fb_bits |= plane->frontbuffer_bit; - } } - intel_frontbuffer_flip(to_i915(dev), fb_bits); + intel_frontbuffer_flip(dev_priv, fb_bits); } static void intel_encoders_pre_pll_enable(struct drm_crtc *crtc, @@ -9897,9 +9906,9 @@ static void i845_update_cursor(struct intel_plane *plane, } static void i845_disable_cursor(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { - i845_update_cursor(plane, NULL, NULL); + i845_update_cursor(plane, crtc_state, NULL); } static bool i845_cursor_get_hw_state(struct intel_plane *plane, @@ -10126,9 +10135,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, } static void i9xx_disable_cursor(struct intel_plane *plane, - struct intel_crtc *crtc) + const struct intel_crtc_state *crtc_state) { - i9xx_update_cursor(plane, NULL, NULL); + i9xx_update_cursor(plane, crtc_state, NULL); } static bool i9xx_cursor_get_hw_state(struct intel_plane *plane, @@ -12892,7 +12901,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state); if (old_crtc_state->active) { - intel_crtc_disable_planes(intel_crtc, old_intel_crtc_state->active_planes); + intel_crtc_disable_planes(intel_state, intel_crtc); /* * We need to disable pipe CRC before disabling the pipe, @@ -13742,7 +13751,7 @@ intel_legacy_cursor_update(struct drm_plane *plane, to_intel_plane_state(plane->state)); } else { trace_intel_disable_plane(plane, to_intel_crtc(crtc)); - intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc)); + intel_plane->disable_plane(intel_plane, crtc_state); } intel_plane_unpin_fb(to_intel_plane_state(old_plane_state)); diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h index 5f2955b944da..a9a181aa4587 100644 --- a/drivers/gpu/drm/i915/intel_display.h +++ b/drivers/gpu/drm/i915/intel_display.h @@ -398,6 +398,14 @@ struct intel_link_m_n { for_each_power_well_reverse(__dev_priv, __power_well) \ for_each_if((__power_well)->desc->domains & (__domain_mask)) +#define for_each_old_intel_plane_in_state(__state, plane, old_plane_state, __i) \ + for ((__i) = 0; \ + (__i) < (__state)->base.dev->mode_config.num_total_plane && \ + ((plane) = to_intel_plane((__state)->base.planes[__i].ptr), \ + (old_plane_state) = to_intel_plane_state((__state)->base.planes[__i].old_state), 1); \ + (__i)++) \ + for_each_if(plane) + #define for_each_new_intel_plane_in_state(__state, plane, new_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->base.dev->mode_config.num_total_plane && \ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1f29061cefa1..7842d193ac44 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1016,7 +1016,7 @@ struct intel_plane { const struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state); void (*disable_plane)(struct intel_plane *plane, - struct intel_crtc *crtc); + const struct intel_crtc_state *crtc_state); bool (*get_hw_state)(struct intel_plane *plane, enum pipe *pipe); int (*check_plane)(struct intel_crtc_state *crtc_state, struct intel_plane_state *plane_state); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 0548b996693f..e5da2fe3bdae 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -594,7 +594,8 @@ icl_update_slave(struct intel_plane *plane, } static void -skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +skl_disable_plane(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; @@ -857,7 +858,8 @@ vlv_update_plane(struct intel_plane *plane, } static void -vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +vlv_disable_plane(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; @@ -1024,7 +1026,8 @@ ivb_update_plane(struct intel_plane *plane, } static void -ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +ivb_disable_plane(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; @@ -1190,7 +1193,8 @@ g4x_update_plane(struct intel_plane *plane, } static void -g4x_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc) +g4x_disable_plane(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = to_i915(plane->base.dev); enum pipe pipe = plane->pipe; -- cgit v1.2.3-55-g7522 From ff43bc379e16c9195323cb88ac0c9f4d0613d07a Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Tue, 27 Nov 2018 18:59:00 +0200 Subject: drm/i915: Move ddb/wm programming into plane update/disable hooks on skl+ On SKL+ the plane WM/BUF_CFG registers are a proper part of each plane's register set. That means accessing them will cancel any pending plane update, and we would need a PLANE_SURF register write to arm the wm/ddb change as well. To avoid all the problems with that let's just move the wm/ddb programming into the plane update/disable hooks. Now all plane registers get written in one (hopefully atomic) operation. To make that feasible we'll move the plane ddb tracking into the crtc state. Watermarks were already tracked there. v2: Rebase due to input CSC v3: Split out a bunch of junk (Matt) v4: Add skl_wm_add_affected_planes() to deal with cursor special case and non-zero wm register reset value v5: Drop the unrelated for_each_intel_plane_mask() fix (Matt) Remove the redundant ddb memset() (Matt) Cc: Matt Roper Cc: Maarten Lankhorst Reviewed-by: Maarten Lankhorst #v3 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181127165900.31298-1-ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 21 +- drivers/gpu/drm/i915/i915_drv.h | 3 - drivers/gpu/drm/i915/intel_display.c | 16 +- drivers/gpu/drm/i915/intel_display.h | 9 + drivers/gpu/drm/i915/intel_drv.h | 9 + drivers/gpu/drm/i915/intel_pm.c | 405 +++++++++++++++++++---------------- drivers/gpu/drm/i915/intel_sprite.c | 4 + 7 files changed, 260 insertions(+), 207 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_sprite.c') diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 596810e0cfe8..129b9a6f8309 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3441,31 +3441,32 @@ static int i915_ddb_info(struct seq_file *m, void *unused) { struct drm_i915_private *dev_priv = node_to_i915(m->private); struct drm_device *dev = &dev_priv->drm; - struct skl_ddb_allocation *ddb; struct skl_ddb_entry *entry; - enum pipe pipe; - int plane; + struct intel_crtc *crtc; if (INTEL_GEN(dev_priv) < 9) return -ENODEV; drm_modeset_lock_all(dev); - ddb = &dev_priv->wm.skl_hw.ddb; - seq_printf(m, "%-15s%8s%8s%8s\n", "", "Start", "End", "Size"); - for_each_pipe(dev_priv, pipe) { + for_each_intel_crtc(&dev_priv->drm, crtc) { + struct intel_crtc_state *crtc_state = + to_intel_crtc_state(crtc->base.state); + enum pipe pipe = crtc->pipe; + enum plane_id plane_id; + seq_printf(m, "Pipe %c\n", pipe_name(pipe)); - for_each_universal_plane(dev_priv, pipe, plane) { - entry = &ddb->plane[pipe][plane]; - seq_printf(m, " Plane%-8d%8u%8u%8u\n", plane + 1, + for_each_plane_id_on_crtc(crtc, plane_id) { + entry = &crtc_state->wm.skl.plane_ddb_y[plane_id]; + seq_printf(m, " Plane%-8d%8u%8u%8u\n", plane_id + 1, entry->start, entry->end, skl_ddb_entry_size(entry)); } - entry = &ddb->plane[pipe][PLANE_CURSOR]; + entry = &crtc_state->wm.skl.plane_ddb_y[PLANE_CURSOR]; seq_printf(m, " %-13s%8u%8u%8u\n", "Cursor", entry->start, entry->end, skl_ddb_entry_size(entry)); } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f763b30f98d9..645c2bbdcdfa 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1095,9 +1095,6 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1, } struct skl_ddb_allocation { - /* packed/y */ - struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; - struct skl_ddb_entry uv_plane[I915_MAX_PIPES][I915_MAX_PLANES]; u8 enabled_slices; /* GEN11 has configurable 2 slices */ }; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d088ffd9f542..6c5d71b44740 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10114,6 +10114,10 @@ static void i9xx_update_cursor(struct intel_plane *plane, * except when the plane is getting enabled at which time * the CURCNTR write arms the update. */ + + if (INTEL_GEN(dev_priv) >= 9) + skl_write_cursor_wm(plane, crtc_state); + if (plane->cursor.base != base || plane->cursor.size != fbc_ctl || plane->cursor.cntl != cntl) { @@ -11903,6 +11907,8 @@ static void verify_wm_state(struct drm_crtc *crtc, struct skl_pipe_wm hw_wm, *sw_wm; struct skl_plane_wm *hw_plane_wm, *sw_plane_wm; struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry; + struct skl_ddb_entry hw_ddb_y[I915_MAX_PLANES]; + struct skl_ddb_entry hw_ddb_uv[I915_MAX_PLANES]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); const enum pipe pipe = intel_crtc->pipe; int plane, level, max_level = ilk_wm_max_level(dev_priv); @@ -11913,6 +11919,8 @@ static void verify_wm_state(struct drm_crtc *crtc, skl_pipe_wm_get_hw_state(crtc, &hw_wm); sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal; + skl_pipe_ddb_get_hw_state(intel_crtc, hw_ddb_y, hw_ddb_uv); + skl_ddb_get_hw_state(dev_priv, &hw_ddb); sw_ddb = &dev_priv->wm.skl_hw.ddb; @@ -11955,8 +11963,8 @@ static void verify_wm_state(struct drm_crtc *crtc, } /* DDB */ - hw_ddb_entry = &hw_ddb.plane[pipe][plane]; - sw_ddb_entry = &sw_ddb->plane[pipe][plane]; + hw_ddb_entry = &hw_ddb_y[plane]; + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane]; if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { DRM_ERROR("mismatch in DDB state pipe %c plane %d (expected (%u,%u), found (%u,%u))\n", @@ -12005,8 +12013,8 @@ static void verify_wm_state(struct drm_crtc *crtc, } /* DDB */ - hw_ddb_entry = &hw_ddb.plane[pipe][PLANE_CURSOR]; - sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR]; + hw_ddb_entry = &hw_ddb_y[PLANE_CURSOR]; + sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR]; if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) { DRM_ERROR("mismatch in DDB state pipe %c cursor (expected (%u,%u), found (%u,%u))\n", diff --git a/drivers/gpu/drm/i915/intel_display.h b/drivers/gpu/drm/i915/intel_display.h index a9a181aa4587..a7ceb8f904f7 100644 --- a/drivers/gpu/drm/i915/intel_display.h +++ b/drivers/gpu/drm/i915/intel_display.h @@ -431,6 +431,15 @@ struct intel_link_m_n { (__i)++) \ for_each_if(plane) +#define for_each_oldnew_intel_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ + for ((__i) = 0; \ + (__i) < (__state)->base.dev->mode_config.num_crtc && \ + ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \ + (old_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].old_state), \ + (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \ + (__i)++) \ + for_each_if(crtc) + void intel_link_compute_m_n(int bpp, int nlanes, int pixel_clock, int link_clock, struct intel_link_m_n *m_n, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 7842d193ac44..1a3a396862d1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -706,6 +706,8 @@ struct intel_crtc_wm_state { /* gen9+ only needs 1-step wm programming */ struct skl_pipe_wm optimal; struct skl_ddb_entry ddb; + struct skl_ddb_entry plane_ddb_y[I915_MAX_PLANES]; + struct skl_ddb_entry plane_ddb_uv[I915_MAX_PLANES]; } skl; struct { @@ -2185,6 +2187,9 @@ void g4x_wm_get_hw_state(struct drm_device *dev); void vlv_wm_get_hw_state(struct drm_device *dev); void ilk_wm_get_hw_state(struct drm_device *dev); void skl_wm_get_hw_state(struct drm_device *dev); +void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc, + struct skl_ddb_entry *ddb_y, + struct skl_ddb_entry *ddb_uv); void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, struct skl_ddb_allocation *ddb /* out */); void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc, @@ -2199,6 +2204,10 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1, bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, const struct skl_ddb_entry entries[], int num_entries, int ignore_idx); +void skl_write_plane_wm(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state); +void skl_write_cursor_wm(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state); bool ilk_disable_lp_wm(struct drm_device *dev); int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc, struct intel_crtc_state *cstate); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a5891f7a9f9d..b1f30d56b747 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3951,68 +3951,68 @@ static void skl_ddb_get_hw_plane_state(struct drm_i915_private *dev_priv, const enum pipe pipe, const enum plane_id plane_id, - struct skl_ddb_allocation *ddb /* out */) + struct skl_ddb_entry *ddb_y, + struct skl_ddb_entry *ddb_uv) { - u32 val, val2 = 0; - int fourcc, pixel_format; + u32 val, val2; + u32 fourcc = 0; /* Cursor doesn't support NV12/planar, so no extra calculation needed */ if (plane_id == PLANE_CURSOR) { val = I915_READ(CUR_BUF_CFG(pipe)); - skl_ddb_entry_init_from_hw(dev_priv, - &ddb->plane[pipe][plane_id], val); + skl_ddb_entry_init_from_hw(dev_priv, ddb_y, val); return; } val = I915_READ(PLANE_CTL(pipe, plane_id)); /* No DDB allocated for disabled planes */ - if (!(val & PLANE_CTL_ENABLE)) - return; - - pixel_format = val & PLANE_CTL_FORMAT_MASK; - fourcc = skl_format_to_fourcc(pixel_format, - val & PLANE_CTL_ORDER_RGBX, - val & PLANE_CTL_ALPHA_MASK); + if (val & PLANE_CTL_ENABLE) + fourcc = skl_format_to_fourcc(val & PLANE_CTL_FORMAT_MASK, + val & PLANE_CTL_ORDER_RGBX, + val & PLANE_CTL_ALPHA_MASK); - val = I915_READ(PLANE_BUF_CFG(pipe, plane_id)); - if (fourcc == DRM_FORMAT_NV12 && INTEL_GEN(dev_priv) < 11) { + if (INTEL_GEN(dev_priv) >= 11) { + val = I915_READ(PLANE_BUF_CFG(pipe, plane_id)); + skl_ddb_entry_init_from_hw(dev_priv, ddb_y, val); + } else { + val = I915_READ(PLANE_BUF_CFG(pipe, plane_id)); val2 = I915_READ(PLANE_NV12_BUF_CFG(pipe, plane_id)); - skl_ddb_entry_init_from_hw(dev_priv, - &ddb->plane[pipe][plane_id], val2); - skl_ddb_entry_init_from_hw(dev_priv, - &ddb->uv_plane[pipe][plane_id], val); - } else { - skl_ddb_entry_init_from_hw(dev_priv, - &ddb->plane[pipe][plane_id], val); + if (fourcc == DRM_FORMAT_NV12) + swap(val, val2); + + skl_ddb_entry_init_from_hw(dev_priv, ddb_y, val); + skl_ddb_entry_init_from_hw(dev_priv, ddb_uv, val2); } } -void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, - struct skl_ddb_allocation *ddb /* out */) +void skl_pipe_ddb_get_hw_state(struct intel_crtc *crtc, + struct skl_ddb_entry *ddb_y, + struct skl_ddb_entry *ddb_uv) { - struct intel_crtc *crtc; - - memset(ddb, 0, sizeof(*ddb)); - - ddb->enabled_slices = intel_enabled_dbuf_slices_num(dev_priv); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + enum intel_display_power_domain power_domain; + enum pipe pipe = crtc->pipe; + enum plane_id plane_id; - for_each_intel_crtc(&dev_priv->drm, crtc) { - enum intel_display_power_domain power_domain; - enum plane_id plane_id; - enum pipe pipe = crtc->pipe; + power_domain = POWER_DOMAIN_PIPE(pipe); + if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) + return; - power_domain = POWER_DOMAIN_PIPE(pipe); - if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) - continue; + for_each_plane_id_on_crtc(crtc, plane_id) + skl_ddb_get_hw_plane_state(dev_priv, pipe, + plane_id, + &ddb_y[plane_id], + &ddb_uv[plane_id]); - for_each_plane_id_on_crtc(crtc, plane_id) - skl_ddb_get_hw_plane_state(dev_priv, pipe, - plane_id, ddb); + intel_display_power_put(dev_priv, power_domain); +} - intel_display_power_put(dev_priv, power_domain); - } +void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, + struct skl_ddb_allocation *ddb /* out */) +{ + ddb->enabled_slices = intel_enabled_dbuf_slices_num(dev_priv); } /* @@ -4410,7 +4410,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, struct drm_crtc *crtc = cstate->base.crtc; struct drm_i915_private *dev_priv = to_i915(crtc->dev); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb; uint16_t alloc_size, start; uint16_t minimum[I915_MAX_PLANES] = {}; @@ -4423,8 +4422,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, uint16_t total_min_blocks = 0; /* Clear the partitioning for disabled planes. */ - memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe])); - memset(ddb->uv_plane[pipe], 0, sizeof(ddb->uv_plane[pipe])); + memset(cstate->wm.skl.plane_ddb_y, 0, sizeof(cstate->wm.skl.plane_ddb_y)); + memset(cstate->wm.skl.plane_ddb_uv, 0, sizeof(cstate->wm.skl.plane_ddb_uv)); if (WARN_ON(!state)) return 0; @@ -4471,8 +4470,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, } alloc_size -= total_min_blocks; - ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR]; - ddb->plane[pipe][PLANE_CURSOR].end = alloc->end; + cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR]; + cstate->wm.skl.plane_ddb_y[PLANE_CURSOR].end = alloc->end; /* * 2. Distribute the remaining space in proportion to the amount of @@ -4503,8 +4502,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, /* Leave disabled planes at (0,0) */ if (data_rate) { - ddb->plane[pipe][plane_id].start = start; - ddb->plane[pipe][plane_id].end = start + plane_blocks; + cstate->wm.skl.plane_ddb_y[plane_id].start = start; + cstate->wm.skl.plane_ddb_y[plane_id].end = start + plane_blocks; } start += plane_blocks; @@ -4519,8 +4518,8 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, WARN_ON(INTEL_GEN(dev_priv) >= 11 && uv_plane_blocks); if (uv_data_rate) { - ddb->uv_plane[pipe][plane_id].start = start; - ddb->uv_plane[pipe][plane_id].end = + cstate->wm.skl.plane_ddb_uv[plane_id].start = start; + cstate->wm.skl.plane_ddb_uv[plane_id].end = start + uv_plane_blocks; } @@ -4978,16 +4977,13 @@ static void skl_compute_transition_wm(const struct intel_crtc_state *cstate, } } -static int skl_build_plane_wm_single(struct skl_ddb_allocation *ddb, - struct intel_crtc_state *crtc_state, +static int skl_build_plane_wm_single(struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state, enum plane_id plane_id, int color_plane) { - struct intel_plane *plane = to_intel_plane(plane_state->base.plane); struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id]; + u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_y[plane_id]); struct skl_wm_params wm_params; - enum pipe pipe = plane->pipe; - uint16_t ddb_blocks = skl_ddb_entry_size(&ddb->plane[pipe][plane_id]); int ret; ret = skl_compute_plane_wm_params(crtc_state, plane_state, @@ -5005,16 +5001,13 @@ static int skl_build_plane_wm_single(struct skl_ddb_allocation *ddb, return 0; } -static int skl_build_plane_wm_uv(struct skl_ddb_allocation *ddb, - struct intel_crtc_state *crtc_state, +static int skl_build_plane_wm_uv(struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state, enum plane_id plane_id) { - struct intel_plane *plane = to_intel_plane(plane_state->base.plane); struct skl_plane_wm *wm = &crtc_state->wm.skl.optimal.planes[plane_id]; + u16 ddb_blocks = skl_ddb_entry_size(&crtc_state->wm.skl.plane_ddb_uv[plane_id]); struct skl_wm_params wm_params; - enum pipe pipe = plane->pipe; - uint16_t ddb_blocks = skl_ddb_entry_size(&ddb->uv_plane[pipe][plane_id]); int ret; wm->is_planar = true; @@ -5033,8 +5026,7 @@ static int skl_build_plane_wm_uv(struct skl_ddb_allocation *ddb, return 0; } -static int skl_build_plane_wm(struct skl_ddb_allocation *ddb, - struct skl_pipe_wm *pipe_wm, +static int skl_build_plane_wm(struct skl_pipe_wm *pipe_wm, struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { @@ -5046,13 +5038,13 @@ static int skl_build_plane_wm(struct skl_ddb_allocation *ddb, if (!intel_wm_plane_visible(crtc_state, plane_state)) return 0; - ret = skl_build_plane_wm_single(ddb, crtc_state, plane_state, + ret = skl_build_plane_wm_single(crtc_state, plane_state, plane_id, 0); if (ret) return ret; if (fb->format->is_yuv && fb->format->num_planes > 1) { - ret = skl_build_plane_wm_uv(ddb, crtc_state, plane_state, + ret = skl_build_plane_wm_uv(crtc_state, plane_state, plane_id); if (ret) return ret; @@ -5061,8 +5053,7 @@ static int skl_build_plane_wm(struct skl_ddb_allocation *ddb, return 0; } -static int icl_build_plane_wm(struct skl_ddb_allocation *ddb, - struct skl_pipe_wm *pipe_wm, +static int icl_build_plane_wm(struct skl_pipe_wm *pipe_wm, struct intel_crtc_state *crtc_state, const struct intel_plane_state *plane_state) { @@ -5081,17 +5072,17 @@ static int icl_build_plane_wm(struct skl_ddb_allocation *ddb, WARN_ON(!fb->format->is_yuv || fb->format->num_planes == 1); - ret = skl_build_plane_wm_single(ddb, crtc_state, plane_state, + ret = skl_build_plane_wm_single(crtc_state, plane_state, y_plane_id, 0); if (ret) return ret; - ret = skl_build_plane_wm_single(ddb, crtc_state, plane_state, + ret = skl_build_plane_wm_single(crtc_state, plane_state, plane_id, 1); if (ret) return ret; } else if (intel_wm_plane_visible(crtc_state, plane_state)) { - ret = skl_build_plane_wm_single(ddb, crtc_state, plane_state, + ret = skl_build_plane_wm_single(crtc_state, plane_state, plane_id, 0); if (ret) return ret; @@ -5101,7 +5092,6 @@ static int icl_build_plane_wm(struct skl_ddb_allocation *ddb, } static int skl_build_pipe_wm(struct intel_crtc_state *cstate, - struct skl_ddb_allocation *ddb, struct skl_pipe_wm *pipe_wm) { struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev); @@ -5121,10 +5111,10 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate, to_intel_plane_state(pstate); if (INTEL_GEN(dev_priv) >= 11) - ret = icl_build_plane_wm(ddb, pipe_wm, + ret = icl_build_plane_wm(pipe_wm, cstate, intel_pstate); else - ret = skl_build_plane_wm(ddb, pipe_wm, + ret = skl_build_plane_wm(pipe_wm, cstate, intel_pstate); if (ret) return ret; @@ -5140,9 +5130,9 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv, const struct skl_ddb_entry *entry) { if (entry->end) - I915_WRITE(reg, (entry->end - 1) << 16 | entry->start); + I915_WRITE_FW(reg, (entry->end - 1) << 16 | entry->start); else - I915_WRITE(reg, 0); + I915_WRITE_FW(reg, 0); } static void skl_write_wm_level(struct drm_i915_private *dev_priv, @@ -5157,19 +5147,22 @@ static void skl_write_wm_level(struct drm_i915_private *dev_priv, val |= level->plane_res_l << PLANE_WM_LINES_SHIFT; } - I915_WRITE(reg, val); + I915_WRITE_FW(reg, val); } -static void skl_write_plane_wm(struct intel_crtc *intel_crtc, - const struct skl_plane_wm *wm, - const struct skl_ddb_allocation *ddb, - enum plane_id plane_id) +void skl_write_plane_wm(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { - struct drm_crtc *crtc = &intel_crtc->base; - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); int level, max_level = ilk_wm_max_level(dev_priv); - enum pipe pipe = intel_crtc->pipe; + enum plane_id plane_id = plane->id; + enum pipe pipe = plane->pipe; + const struct skl_plane_wm *wm = + &crtc_state->wm.skl.optimal.planes[plane_id]; + const struct skl_ddb_entry *ddb_y = + &crtc_state->wm.skl.plane_ddb_y[plane_id]; + const struct skl_ddb_entry *ddb_uv = + &crtc_state->wm.skl.plane_ddb_uv[plane_id]; for (level = 0; level <= max_level; level++) { skl_write_wm_level(dev_priv, PLANE_WM(pipe, plane_id, level), @@ -5178,29 +5171,32 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc, skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id), &wm->trans_wm); - if (wm->is_planar && INTEL_GEN(dev_priv) < 11) { - skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id), - &ddb->uv_plane[pipe][plane_id]); + if (INTEL_GEN(dev_priv) >= 11) { skl_ddb_entry_write(dev_priv, - PLANE_NV12_BUF_CFG(pipe, plane_id), - &ddb->plane[pipe][plane_id]); - } else { - skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id), - &ddb->plane[pipe][plane_id]); - if (INTEL_GEN(dev_priv) < 11) - I915_WRITE(PLANE_NV12_BUF_CFG(pipe, plane_id), 0x0); + PLANE_BUF_CFG(pipe, plane_id), ddb_y); + return; } + + if (wm->is_planar) + swap(ddb_y, ddb_uv); + + skl_ddb_entry_write(dev_priv, + PLANE_BUF_CFG(pipe, plane_id), ddb_y); + skl_ddb_entry_write(dev_priv, + PLANE_NV12_BUF_CFG(pipe, plane_id), ddb_uv); } -static void skl_write_cursor_wm(struct intel_crtc *intel_crtc, - const struct skl_plane_wm *wm, - const struct skl_ddb_allocation *ddb) +void skl_write_cursor_wm(struct intel_plane *plane, + const struct intel_crtc_state *crtc_state) { - struct drm_crtc *crtc = &intel_crtc->base; - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); int level, max_level = ilk_wm_max_level(dev_priv); - enum pipe pipe = intel_crtc->pipe; + enum plane_id plane_id = plane->id; + enum pipe pipe = plane->pipe; + const struct skl_plane_wm *wm = + &crtc_state->wm.skl.optimal.planes[plane_id]; + const struct skl_ddb_entry *ddb = + &crtc_state->wm.skl.plane_ddb_y[plane_id]; for (level = 0; level <= max_level; level++) { skl_write_wm_level(dev_priv, CUR_WM(pipe, level), @@ -5208,22 +5204,30 @@ static void skl_write_cursor_wm(struct intel_crtc *intel_crtc, } skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm->trans_wm); - skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), - &ddb->plane[pipe][PLANE_CURSOR]); + skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), ddb); } bool skl_wm_level_equals(const struct skl_wm_level *l1, const struct skl_wm_level *l2) { - if (l1->plane_en != l2->plane_en) - return false; + return l1->plane_en == l2->plane_en && + l1->plane_res_l == l2->plane_res_l && + l1->plane_res_b == l2->plane_res_b; +} - /* If both planes aren't enabled, the rest shouldn't matter */ - if (!l1->plane_en) - return true; +static bool skl_plane_wm_equals(struct drm_i915_private *dev_priv, + const struct skl_plane_wm *wm1, + const struct skl_plane_wm *wm2) +{ + int level, max_level = ilk_wm_max_level(dev_priv); - return (l1->plane_res_l == l2->plane_res_l && - l1->plane_res_b == l2->plane_res_b); + for (level = 0; level <= max_level; level++) { + if (!skl_wm_level_equals(&wm1->wm[level], &wm2->wm[level]) || + !skl_wm_level_equals(&wm1->uv_wm[level], &wm2->uv_wm[level])) + return false; + } + + return skl_wm_level_equals(&wm1->trans_wm, &wm2->trans_wm); } static inline bool skl_ddb_entries_overlap(const struct skl_ddb_entry *a, @@ -5250,13 +5254,12 @@ bool skl_ddb_allocation_overlaps(const struct skl_ddb_entry *ddb, static int skl_update_pipe_wm(struct drm_crtc_state *cstate, const struct skl_pipe_wm *old_pipe_wm, struct skl_pipe_wm *pipe_wm, /* out */ - struct skl_ddb_allocation *ddb, /* out */ bool *changed /* out */) { struct intel_crtc_state *intel_cstate = to_intel_crtc_state(cstate); int ret; - ret = skl_build_pipe_wm(intel_cstate, ddb, pipe_wm); + ret = skl_build_pipe_wm(intel_cstate, pipe_wm); if (ret) return ret; @@ -5282,42 +5285,29 @@ pipes_modified(struct drm_atomic_state *state) } static int -skl_ddb_add_affected_planes(struct intel_crtc_state *cstate) +skl_ddb_add_affected_planes(const struct intel_crtc_state *old_crtc_state, + struct intel_crtc_state *new_crtc_state) { - struct drm_atomic_state *state = cstate->base.state; - struct drm_device *dev = state->dev; - struct drm_crtc *crtc = cstate->base.crtc; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_atomic_state *intel_state = to_intel_atomic_state(state); - struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb; - struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb; - struct drm_plane *plane; - enum pipe pipe = intel_crtc->pipe; + struct intel_atomic_state *state = to_intel_atomic_state(new_crtc_state->base.state); + struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + struct intel_plane *plane; - drm_for_each_plane_mask(plane, dev, cstate->base.plane_mask) { - struct drm_plane_state *plane_state; - struct intel_plane *linked; - enum plane_id plane_id = to_intel_plane(plane)->id; + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) { + struct intel_plane_state *plane_state; + enum plane_id plane_id = plane->id; - if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][plane_id], - &new_ddb->plane[pipe][plane_id]) && - skl_ddb_entry_equal(&cur_ddb->uv_plane[pipe][plane_id], - &new_ddb->uv_plane[pipe][plane_id])) + if (skl_ddb_entry_equal(&old_crtc_state->wm.skl.plane_ddb_y[plane_id], + &new_crtc_state->wm.skl.plane_ddb_y[plane_id]) && + skl_ddb_entry_equal(&old_crtc_state->wm.skl.plane_ddb_uv[plane_id], + &new_crtc_state->wm.skl.plane_ddb_uv[plane_id])) continue; - plane_state = drm_atomic_get_plane_state(state, plane); + plane_state = intel_atomic_get_plane_state(state, plane); if (IS_ERR(plane_state)) return PTR_ERR(plane_state); - /* Make sure linked plane is updated too */ - linked = to_intel_plane_state(plane_state)->linked_plane; - if (!linked) - continue; - - plane_state = drm_atomic_get_plane_state(state, &linked->base); - if (IS_ERR(plane_state)) - return PTR_ERR(plane_state); + new_crtc_state->update_planes |= BIT(plane_id); } return 0; @@ -5329,18 +5319,21 @@ skl_compute_ddb(struct drm_atomic_state *state) const struct drm_i915_private *dev_priv = to_i915(state->dev); struct intel_atomic_state *intel_state = to_intel_atomic_state(state); struct skl_ddb_allocation *ddb = &intel_state->wm_results.ddb; + struct intel_crtc_state *old_crtc_state; + struct intel_crtc_state *new_crtc_state; struct intel_crtc *crtc; - struct intel_crtc_state *cstate; int ret, i; memcpy(ddb, &dev_priv->wm.skl_hw.ddb, sizeof(*ddb)); - for_each_new_intel_crtc_in_state(intel_state, crtc, cstate, i) { - ret = skl_allocate_pipe_ddb(cstate, ddb); + for_each_oldnew_intel_crtc_in_state(intel_state, crtc, old_crtc_state, + new_crtc_state, i) { + ret = skl_allocate_pipe_ddb(new_crtc_state, ddb); if (ret) return ret; - ret = skl_ddb_add_affected_planes(cstate); + ret = skl_ddb_add_affected_planes(old_crtc_state, + new_crtc_state); if (ret) return ret; } @@ -5349,36 +5342,29 @@ skl_compute_ddb(struct drm_atomic_state *state) } static void -skl_print_wm_changes(const struct drm_atomic_state *state) +skl_print_wm_changes(struct intel_atomic_state *state) { - const struct drm_device *dev = state->dev; - const struct drm_i915_private *dev_priv = to_i915(dev); - const struct intel_atomic_state *intel_state = - to_intel_atomic_state(state); - const struct drm_crtc *crtc; - const struct drm_crtc_state *cstate; - const struct intel_plane *intel_plane; - const struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb; - const struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb; + struct drm_i915_private *dev_priv = to_i915(state->base.dev); + const struct intel_crtc_state *old_crtc_state; + const struct intel_crtc_state *new_crtc_state; + struct intel_plane *plane; + struct intel_crtc *crtc; int i; - for_each_new_crtc_in_state(state, crtc, cstate, i) { - const struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { - enum plane_id plane_id = intel_plane->id; + for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, + new_crtc_state, i) { + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) { + enum plane_id plane_id = plane->id; const struct skl_ddb_entry *old, *new; - old = &old_ddb->plane[pipe][plane_id]; - new = &new_ddb->plane[pipe][plane_id]; + old = &old_crtc_state->wm.skl.plane_ddb_y[plane_id]; + new = &new_crtc_state->wm.skl.plane_ddb_y[plane_id]; if (skl_ddb_entry_equal(old, new)) continue; DRM_DEBUG_KMS("[PLANE:%d:%s] ddb (%d - %d) -> (%d - %d)\n", - intel_plane->base.base.id, - intel_plane->base.name, + plane->base.base.id, plane->base.name, old->start, old->end, new->start, new->end); } @@ -5475,6 +5461,66 @@ skl_ddb_add_affected_pipes(struct drm_atomic_state *state, bool *changed) return 0; } +/* + * To make sure the cursor watermark registers are always consistent + * with our computed state the following scenario needs special + * treatment: + * + * 1. enable cursor + * 2. move cursor entirely offscreen + * 3. disable cursor + * + * Step 2. does call .disable_plane() but does not zero the watermarks + * (since we consider an offscreen cursor still active for the purposes + * of watermarks). Step 3. would not normally call .disable_plane() + * because the actual plane visibility isn't changing, and we don't + * deallocate the cursor ddb until the pipe gets disabled. So we must + * force step 3. to call .disable_plane() to update the watermark + * registers properly. + * + * Other planes do not suffer from this issues as their watermarks are + * calculated based on the actual plane visibility. The only time this + * can trigger for the other planes is during the initial readout as the + * default value of the watermarks registers is not zero. + */ +static int skl_wm_add_affected_planes(struct intel_atomic_state *state, + struct intel_crtc *crtc) +{ + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + const struct intel_crtc_state *old_crtc_state = + intel_atomic_get_old_crtc_state(state, crtc); + struct intel_crtc_state *new_crtc_state = + intel_atomic_get_new_crtc_state(state, crtc); + struct intel_plane *plane; + + for_each_intel_plane_on_crtc(&dev_priv->drm, crtc, plane) { + struct intel_plane_state *plane_state; + enum plane_id plane_id = plane->id; + + /* + * Force a full wm update for every plane on modeset. + * Required because the reset value of the wm registers + * is non-zero, whereas we want all disabled planes to + * have zero watermarks. So if we turn off the relevant + * power well the hardware state will go out of sync + * with the software state. + */ + if (!drm_atomic_crtc_needs_modeset(&new_crtc_state->base) && + skl_plane_wm_equals(dev_priv, + &old_crtc_state->wm.skl.optimal.planes[plane_id], + &new_crtc_state->wm.skl.optimal.planes[plane_id])) + continue; + + plane_state = intel_atomic_get_plane_state(state, plane); + if (IS_ERR(plane_state)) + return PTR_ERR(plane_state); + + new_crtc_state->update_planes |= BIT(plane_id); + } + + return 0; +} + static int skl_compute_wm(struct drm_atomic_state *state) { @@ -5514,8 +5560,12 @@ skl_compute_wm(struct drm_atomic_state *state) &to_intel_crtc_state(crtc->state)->wm.skl.optimal; pipe_wm = &intel_cstate->wm.skl.optimal; - ret = skl_update_pipe_wm(cstate, old_pipe_wm, pipe_wm, - &results->ddb, &changed); + ret = skl_update_pipe_wm(cstate, old_pipe_wm, pipe_wm, &changed); + if (ret) + return ret; + + ret = skl_wm_add_affected_planes(intel_state, + to_intel_crtc(crtc)); if (ret) return ret; @@ -5529,7 +5579,7 @@ skl_compute_wm(struct drm_atomic_state *state) intel_cstate->update_wm_pre = true; } - skl_print_wm_changes(state); + skl_print_wm_changes(intel_state); return 0; } @@ -5540,23 +5590,12 @@ static void skl_atomic_update_crtc_wm(struct intel_atomic_state *state, struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc); struct drm_i915_private *dev_priv = to_i915(state->base.dev); struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal; - const struct skl_ddb_allocation *ddb = &state->wm_results.ddb; enum pipe pipe = crtc->pipe; - enum plane_id plane_id; if (!(state->wm_results.dirty_pipes & drm_crtc_mask(&crtc->base))) return; I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime); - - for_each_plane_id_on_crtc(crtc, plane_id) { - if (plane_id != PLANE_CURSOR) - skl_write_plane_wm(crtc, &pipe_wm->planes[plane_id], - ddb, plane_id); - else - skl_write_cursor_wm(crtc, &pipe_wm->planes[plane_id], - ddb); - } } static void skl_initial_wm(struct intel_atomic_state *state, @@ -5566,8 +5605,6 @@ static void skl_initial_wm(struct intel_atomic_state *state, struct drm_device *dev = intel_crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); struct skl_ddb_values *results = &state->wm_results; - struct skl_ddb_values *hw_vals = &dev_priv->wm.skl_hw; - enum pipe pipe = intel_crtc->pipe; if ((results->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) == 0) return; @@ -5577,11 +5614,6 @@ static void skl_initial_wm(struct intel_atomic_state *state, if (cstate->base.active_changed) skl_atomic_update_crtc_wm(state, cstate); - memcpy(hw_vals->ddb.uv_plane[pipe], results->ddb.uv_plane[pipe], - sizeof(hw_vals->ddb.uv_plane[pipe])); - memcpy(hw_vals->ddb.plane[pipe], results->ddb.plane[pipe], - sizeof(hw_vals->ddb.plane[pipe])); - mutex_unlock(&dev_priv->wm.wm_mutex); } @@ -5732,13 +5764,6 @@ void skl_wm_get_hw_state(struct drm_device *dev) if (dev_priv->active_crtcs) { /* Fully recompute DDB on first atomic commit */ dev_priv->wm.distrust_bios_wm = true; - } else { - /* - * Easy/common case; just sanitize DDB now if everything off - * Keep dbuf slice info intact - */ - memset(ddb->plane, 0, sizeof(ddb->plane)); - memset(ddb->uv_plane, 0, sizeof(ddb->uv_plane)); } } diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index e5da2fe3bdae..32df604e90b6 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -542,6 +542,8 @@ skl_program_plane(struct intel_plane *plane, if (fb->format->is_yuv && icl_is_hdr_plane(plane)) icl_program_input_csc_coeff(crtc_state, plane_state); + skl_write_plane_wm(plane, crtc_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); @@ -604,6 +606,8 @@ skl_disable_plane(struct intel_plane *plane, spin_lock_irqsave(&dev_priv->uncore.lock, irqflags); + skl_write_plane_wm(plane, crtc_state); + I915_WRITE_FW(PLANE_CTL(pipe, plane_id), 0); I915_WRITE_FW(PLANE_SURF(pipe, plane_id), 0); -- cgit v1.2.3-55-g7522 From 1fdee7582cce4c9d56b24e6caf1a87cd47a11825 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 14 Nov 2018 23:07:29 +0200 Subject: drm/i915: Pass the plane to icl_program_input_csc_coeff() On icl+ the plane state that gets passed to update_slave() is not the plane state of the plane we're programming. With NV12 the plane state would be coming from the master (UV) plane whereas the plane we're programming is the slave (Y) plane. For that reason we need to explicitly pass around the slave plane (or we'd have to otherwise deduce it by checking whether we were called via .update_plane() or .update_slave()). In the case of icl_program_input_csc_coeff() it's actually OK to assume that we are always the master plane because the input CSC only exists on HDR planes which can never be a slave plane. But for consistency let's pass in the plane explicitly anyway. While at it drop the "_coeff" from the function name since it's kinda redundant, and this makes the name a bit shorter :) Cc: Uma Shankar Cc: Maarten Lankhorst Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20181114210729.16185-14-ville.syrjala@linux.intel.com Reviewed-by: Uma Shankar Reviewed-by: Maarten Lankhorst Reviewed-by: Matt Roper --- drivers/gpu/drm/i915/intel_sprite.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_sprite.c') diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 32df604e90b6..d2e003d8f3db 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -373,14 +373,12 @@ skl_program_scaler(struct intel_plane *plane, #define BOFF(x) (((x) & 0xffff) << 16) static void -icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state, - const struct intel_plane_state *plane_state) +icl_program_input_csc(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_state->base.plane->dev); - struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); - enum pipe pipe = crtc->pipe; - struct intel_plane *plane = to_intel_plane(plane_state->base.plane); + struct drm_i915_private *dev_priv = to_i915(plane->base.dev); + enum pipe pipe = plane->pipe; enum plane_id plane_id = plane->id; static const u16 input_csc_matrix[][9] = { @@ -540,7 +538,7 @@ skl_program_plane(struct intel_plane *plane, plane_state->color_ctl); if (fb->format->is_yuv && icl_is_hdr_plane(plane)) - icl_program_input_csc_coeff(crtc_state, plane_state); + icl_program_input_csc(plane, crtc_state, plane_state); skl_write_plane_wm(plane, crtc_state); -- cgit v1.2.3-55-g7522