| Message ID | 20160909080106.17506-9-mahesh1.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Sex, 2016-09-09 às 13:31 +0530, Kumar, Mahesh escreveu: > From: Mahesh Kumar <mahesh1.kumar@intel.com> > > It implements the WA to enable IDLE_WAKEMEM bit of CHICKEN_DCPR_1 > register for Broxton platform. When IPC is enabled & Y-tile is > enabled in any of the enabled plane, above bit should be set. > Without this WA system observes random hang. The previous patch enabled the feature, and now this patch implements a missing workaround for the feature. This is not the appropriate order, since it can mean that the previous patch introduced a bug that was fixed by this patch, and this potentially breaks bisectability. We only enable the feature once we know all its workarounds are enabled and the feature will Just Work*. So, normally, my suggestion would be to either invert the patch order, or just make patches 7 and 8 be a single patch. But we have another problem, please see commit 590e8ff04bc0182dce97228e5e352d6413d80456. What's the problem with the current implementation? Why can't we just keep the bit as 1 forever? > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 50 > ++++++++++++++++++++++++++++++++++++ > 3 files changed, 55 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index 4737a0e..79b9305 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1632,6 +1632,8 @@ struct skl_wm_values { > unsigned dirty_pipes; > /* any WaterMark memory workaround Required */ > enum watermark_memory_wa mem_wa; > + /* IPC Y-tiled WA related member */ > + u32 y_plane_mask; > struct skl_ddb_allocation ddb; > uint32_t wm_linetime[I915_MAX_PIPES]; > uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8]; > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index 75b5b52..210d9b0 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5658,6 +5658,9 @@ enum { > #define PLANE_NV12_BUF_CFG(pipe, plane) \ > _MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), > _PLANE_NV12_BUF_CFG_2(pipe)) > > +#define CHICKEN_DCPR_1 _MMIO(0x46430) > +#define IDLE_WAKEMEM_MASK (1 << 13) > + > /* SKL new cursor registers */ > #define _CUR_BUF_CFG_A 0x7017c > #define _CUR_BUF_CFG_B 0x7117c > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 5f50f53..ee7f88e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12415,6 +12415,8 @@ int intel_plane_atomic_calc_changes(struct > drm_crtc_state *crtc_state, > bool is_crtc_enabled = crtc_state->active; > bool turn_off, turn_on, visible, was_visible; > struct drm_framebuffer *fb = plane_state->fb; > + struct intel_atomic_state *intel_state = > + to_intel_atomic_state(plane_state- > >state); > int ret; > > if (INTEL_GEN(dev) >= 9 && plane->type != > DRM_PLANE_TYPE_CURSOR) { > @@ -12501,6 +12503,15 @@ int intel_plane_atomic_calc_changes(struct > drm_crtc_state *crtc_state, > !needs_scaling(old_plane_state)) > pipe_config->disable_lp_wm = true; > > + if (fb && (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || > + fb->modifier[0] == > I915_FORMAT_MOD_Yf_TILED)) { > + intel_state->wm_results.y_plane_mask |= > + (1 << > drm_plane_index(plane)); > + } else { > + intel_state->wm_results.y_plane_mask &= > + ~(1 << > drm_plane_index(plane)); > + } > + > return 0; > } > > @@ -13963,6 +13974,10 @@ static int intel_atomic_check(struct > drm_device *dev, > if (ret) > return ret; > > + /* Copy the Y-tile WA related states */ > + intel_state->wm_results.y_plane_mask = > + dev_priv- > >wm.skl_results.y_plane_mask; > + > for_each_crtc_in_state(state, crtc, crtc_state, i) { > struct intel_crtc_state *pipe_config = > to_intel_crtc_state(crtc_state); > @@ -14204,6 +14219,32 @@ static void intel_update_crtcs(struct > drm_atomic_state *state, > } > } > > +/* > + * GEN9 IPC WA for Y-tiled > + */ > +void bxt_set_ipc_wa(struct drm_i915_private *dev_priv, bool enable) > +{ > + u32 val; > + > + if (!IS_BROXTON(dev_priv) || !i915.enable_ipc) > + return; > + > + val = I915_READ(CHICKEN_DCPR_1); > + /* > + * If WA is already enabled or disabled > + * no need to re-enable or disable it. > + */ > + if ((enable && (val & IDLE_WAKEMEM_MASK)) || > + (!enable && !(val & IDLE_WAKEMEM_MASK))) > + return; > + > + if (enable) > + val |= IDLE_WAKEMEM_MASK; > + else > + val &= ~IDLE_WAKEMEM_MASK; > + I915_WRITE(CHICKEN_DCPR_1, val); > +} > + > static void skl_update_crtcs(struct drm_atomic_state *state, > unsigned int *crtc_vblank_mask) > { > @@ -14219,6 +14260,12 @@ static void skl_update_crtcs(struct > drm_atomic_state *state, > enum pipe pipe; > > /* > + * If IPC WA need to be enabled, enable it now > + */ > + if (intel_state->wm_results.y_plane_mask) > + bxt_set_ipc_wa(dev_priv, true); > + > + /* > * Whenever the number of active pipes changes, we need to > make sure we > * update the pipes in the right order so that their ddb > allocations > * never overlap with eachother inbetween CRTC updates. > Otherwise we'll > @@ -14261,6 +14308,9 @@ static void skl_update_crtcs(struct > drm_atomic_state *state, > progress = true; > } > } while (progress); > + > + if (!intel_state->wm_results.y_plane_mask) > + bxt_set_ipc_wa(dev_priv, false); > } > > static void intel_atomic_commit_tail(struct drm_atomic_state *state)
Hi, On Thursday 22 September 2016 01:53 AM, Paulo Zanoni wrote: > Em Sex, 2016-09-09 às 13:31 +0530, Kumar, Mahesh escreveu: >> From: Mahesh Kumar <mahesh1.kumar@intel.com> >> >> It implements the WA to enable IDLE_WAKEMEM bit of CHICKEN_DCPR_1 >> register for Broxton platform. When IPC is enabled & Y-tile is >> enabled in any of the enabled plane, above bit should be set. >> Without this WA system observes random hang. > The previous patch enabled the feature, and now this patch implements a > missing workaround for the feature. This is not the appropriate order, > since it can mean that the previous patch introduced a bug that was > fixed by this patch, and this potentially breaks bisectability. We only > enable the feature once we know all its workarounds are enabled and the > feature will Just Work*. So, normally, my suggestion would be to either > invert the patch order, or just make patches 7 and 8 be a single patch. I can invert the order of patches, If below point seems OK. > > But we have another problem, please see > commit 590e8ff04bc0182dce97228e5e352d6413d80456. What's the problem > with the current implementation? Why can't we just keep the bit as 1 > forever? mentioned commit is making IDLE_WAKEMEM bit always 1b (masking), but it is require only in case of y/yf tiling. need to check for consequences in keeping this bit always Masked. what is your opinion on this? Should not we implement it only in case of y/yf tiling, when we can do it in our framework? Regards, -Mahesh > > >> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> drivers/gpu/drm/i915/i915_reg.h | 3 +++ >> drivers/gpu/drm/i915/intel_display.c | 50 >> ++++++++++++++++++++++++++++++++++++ >> 3 files changed, 55 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h >> b/drivers/gpu/drm/i915/i915_drv.h >> index 4737a0e..79b9305 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -1632,6 +1632,8 @@ struct skl_wm_values { >> unsigned dirty_pipes; >> /* any WaterMark memory workaround Required */ >> enum watermark_memory_wa mem_wa; >> + /* IPC Y-tiled WA related member */ >> + u32 y_plane_mask; >> struct skl_ddb_allocation ddb; >> uint32_t wm_linetime[I915_MAX_PIPES]; >> uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8]; >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h >> index 75b5b52..210d9b0 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -5658,6 +5658,9 @@ enum { >> #define PLANE_NV12_BUF_CFG(pipe, plane) \ >> _MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), >> _PLANE_NV12_BUF_CFG_2(pipe)) >> >> +#define CHICKEN_DCPR_1 _MMIO(0x46430) >> +#define IDLE_WAKEMEM_MASK (1 << 13) >> + >> /* SKL new cursor registers */ >> #define _CUR_BUF_CFG_A 0x7017c >> #define _CUR_BUF_CFG_B 0x7117c >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 5f50f53..ee7f88e 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -12415,6 +12415,8 @@ int intel_plane_atomic_calc_changes(struct >> drm_crtc_state *crtc_state, >> bool is_crtc_enabled = crtc_state->active; >> bool turn_off, turn_on, visible, was_visible; >> struct drm_framebuffer *fb = plane_state->fb; >> + struct intel_atomic_state *intel_state = >> + to_intel_atomic_state(plane_state- >>> state); >> int ret; >> >> if (INTEL_GEN(dev) >= 9 && plane->type != >> DRM_PLANE_TYPE_CURSOR) { >> @@ -12501,6 +12503,15 @@ int intel_plane_atomic_calc_changes(struct >> drm_crtc_state *crtc_state, >> !needs_scaling(old_plane_state)) >> pipe_config->disable_lp_wm = true; >> >> + if (fb && (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || >> + fb->modifier[0] == >> I915_FORMAT_MOD_Yf_TILED)) { >> + intel_state->wm_results.y_plane_mask |= >> + (1 << >> drm_plane_index(plane)); >> + } else { >> + intel_state->wm_results.y_plane_mask &= >> + ~(1 << >> drm_plane_index(plane)); >> + } >> + >> return 0; >> } >> >> @@ -13963,6 +13974,10 @@ static int intel_atomic_check(struct >> drm_device *dev, >> if (ret) >> return ret; >> >> + /* Copy the Y-tile WA related states */ >> + intel_state->wm_results.y_plane_mask = >> + dev_priv- >>> wm.skl_results.y_plane_mask; >> + >> for_each_crtc_in_state(state, crtc, crtc_state, i) { >> struct intel_crtc_state *pipe_config = >> to_intel_crtc_state(crtc_state); >> @@ -14204,6 +14219,32 @@ static void intel_update_crtcs(struct >> drm_atomic_state *state, >> } >> } >> >> +/* >> + * GEN9 IPC WA for Y-tiled >> + */ >> +void bxt_set_ipc_wa(struct drm_i915_private *dev_priv, bool enable) >> +{ >> + u32 val; >> + >> + if (!IS_BROXTON(dev_priv) || !i915.enable_ipc) >> + return; >> + >> + val = I915_READ(CHICKEN_DCPR_1); >> + /* >> + * If WA is already enabled or disabled >> + * no need to re-enable or disable it. >> + */ >> + if ((enable && (val & IDLE_WAKEMEM_MASK)) || >> + (!enable && !(val & IDLE_WAKEMEM_MASK))) >> + return; >> + >> + if (enable) >> + val |= IDLE_WAKEMEM_MASK; >> + else >> + val &= ~IDLE_WAKEMEM_MASK; >> + I915_WRITE(CHICKEN_DCPR_1, val); >> +} >> + >> static void skl_update_crtcs(struct drm_atomic_state *state, >> unsigned int *crtc_vblank_mask) >> { >> @@ -14219,6 +14260,12 @@ static void skl_update_crtcs(struct >> drm_atomic_state *state, >> enum pipe pipe; >> >> /* >> + * If IPC WA need to be enabled, enable it now >> + */ >> + if (intel_state->wm_results.y_plane_mask) >> + bxt_set_ipc_wa(dev_priv, true); >> + >> + /* >> * Whenever the number of active pipes changes, we need to >> make sure we >> * update the pipes in the right order so that their ddb >> allocations >> * never overlap with eachother inbetween CRTC updates. >> Otherwise we'll >> @@ -14261,6 +14308,9 @@ static void skl_update_crtcs(struct >> drm_atomic_state *state, >> progress = true; >> } >> } while (progress); >> + >> + if (!intel_state->wm_results.y_plane_mask) >> + bxt_set_ipc_wa(dev_priv, false); >> } >> >> static void intel_atomic_commit_tail(struct drm_atomic_state *state)
Em Qui, 2016-09-22 às 15:13 +0530, Mahesh Kumar escreveu: > Hi, > > > On Thursday 22 September 2016 01:53 AM, Paulo Zanoni wrote: > > > > Em Sex, 2016-09-09 às 13:31 +0530, Kumar, Mahesh escreveu: > > > > > > From: Mahesh Kumar <mahesh1.kumar@intel.com> > > > > > > It implements the WA to enable IDLE_WAKEMEM bit of CHICKEN_DCPR_1 > > > register for Broxton platform. When IPC is enabled & Y-tile is > > > enabled in any of the enabled plane, above bit should be set. > > > Without this WA system observes random hang. > > The previous patch enabled the feature, and now this patch > > implements a > > missing workaround for the feature. This is not the appropriate > > order, > > since it can mean that the previous patch introduced a bug that was > > fixed by this patch, and this potentially breaks bisectability. We > > only > > enable the feature once we know all its workarounds are enabled and > > the > > feature will Just Work*. So, normally, my suggestion would be to > > either > > invert the patch order, or just make patches 7 and 8 be a single > > patch. > I can invert the order of patches, If below point seems OK. > > > > > > But we have another problem, please see > > commit 590e8ff04bc0182dce97228e5e352d6413d80456. What's the problem > > with the current implementation? Why can't we just keep the bit as > > 1 > > forever? > mentioned commit is making IDLE_WAKEMEM bit always 1b (masking), but > it > is require only in case of y/yf tiling. > need to check for consequences in keeping this bit always Masked. > what is your opinion on this? Should not we implement it only in case > of > y/yf tiling, when we can do it in our framework? If there are no bad consequences for keeping the bit as 1 even when we're not using y/yf tiling, then the simpler solution wins IMHO. > > Regards, > -Mahesh > > > > > > > > > > > > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > > > drivers/gpu/drm/i915/intel_display.c | 50 > > > ++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 55 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > b/drivers/gpu/drm/i915/i915_drv.h > > > index 4737a0e..79b9305 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > @@ -1632,6 +1632,8 @@ struct skl_wm_values { > > > unsigned dirty_pipes; > > > /* any WaterMark memory workaround Required */ > > > enum watermark_memory_wa mem_wa; > > > + /* IPC Y-tiled WA related member */ > > > + u32 y_plane_mask; > > > struct skl_ddb_allocation ddb; > > > uint32_t wm_linetime[I915_MAX_PIPES]; > > > uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8]; > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > b/drivers/gpu/drm/i915/i915_reg.h > > > index 75b5b52..210d9b0 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -5658,6 +5658,9 @@ enum { > > > #define PLANE_NV12_BUF_CFG(pipe, plane) \ > > > _MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), > > > _PLANE_NV12_BUF_CFG_2(pipe)) > > > > > > +#define CHICKEN_DCPR_1 _MMIO(0x46430) > > > +#define IDLE_WAKEMEM_MASK (1 << 13) > > > + > > > /* SKL new cursor registers */ > > > #define _CUR_BUF_CFG_A 0x7017c > > > #define _CUR_BUF_CFG_B 0x7117c > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index 5f50f53..ee7f88e 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -12415,6 +12415,8 @@ int > > > intel_plane_atomic_calc_changes(struct > > > drm_crtc_state *crtc_state, > > > bool is_crtc_enabled = crtc_state->active; > > > bool turn_off, turn_on, visible, was_visible; > > > struct drm_framebuffer *fb = plane_state->fb; > > > + struct intel_atomic_state *intel_state = > > > + to_intel_atomic_state(plane_stat > > > e- > > > > > > > > state); > > > int ret; > > > > > > if (INTEL_GEN(dev) >= 9 && plane->type != > > > DRM_PLANE_TYPE_CURSOR) { > > > @@ -12501,6 +12503,15 @@ int > > > intel_plane_atomic_calc_changes(struct > > > drm_crtc_state *crtc_state, > > > !needs_scaling(old_plane_state)) > > > pipe_config->disable_lp_wm = true; > > > > > > + if (fb && (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || > > > + fb->modifier[0] == > > > I915_FORMAT_MOD_Yf_TILED)) { > > > + intel_state->wm_results.y_plane_mask |= > > > + (1 << > > > drm_plane_index(plane)); > > > + } else { > > > + intel_state->wm_results.y_plane_mask &= > > > + ~(1 << > > > drm_plane_index(plane)); > > > + } > > > + > > > return 0; > > > } > > > > > > @@ -13963,6 +13974,10 @@ static int intel_atomic_check(struct > > > drm_device *dev, > > > if (ret) > > > return ret; > > > > > > + /* Copy the Y-tile WA related states */ > > > + intel_state->wm_results.y_plane_mask = > > > + dev_priv- > > > > > > > > wm.skl_results.y_plane_mask; > > > + > > > for_each_crtc_in_state(state, crtc, crtc_state, i) { > > > struct intel_crtc_state *pipe_config = > > > to_intel_crtc_state(crtc_state); > > > @@ -14204,6 +14219,32 @@ static void intel_update_crtcs(struct > > > drm_atomic_state *state, > > > } > > > } > > > > > > +/* > > > + * GEN9 IPC WA for Y-tiled > > > + */ > > > +void bxt_set_ipc_wa(struct drm_i915_private *dev_priv, bool > > > enable) > > > +{ > > > + u32 val; > > > + > > > + if (!IS_BROXTON(dev_priv) || !i915.enable_ipc) > > > + return; > > > + > > > + val = I915_READ(CHICKEN_DCPR_1); > > > + /* > > > + * If WA is already enabled or disabled > > > + * no need to re-enable or disable it. > > > + */ > > > + if ((enable && (val & IDLE_WAKEMEM_MASK)) || > > > + (!enable && !(val & IDLE_WAKEMEM_MASK))) > > > + return; > > > + > > > + if (enable) > > > + val |= IDLE_WAKEMEM_MASK; > > > + else > > > + val &= ~IDLE_WAKEMEM_MASK; > > > + I915_WRITE(CHICKEN_DCPR_1, val); > > > +} > > > + > > > static void skl_update_crtcs(struct drm_atomic_state *state, > > > unsigned int *crtc_vblank_mask) > > > { > > > @@ -14219,6 +14260,12 @@ static void skl_update_crtcs(struct > > > drm_atomic_state *state, > > > enum pipe pipe; > > > > > > /* > > > + * If IPC WA need to be enabled, enable it now > > > + */ > > > + if (intel_state->wm_results.y_plane_mask) > > > + bxt_set_ipc_wa(dev_priv, true); > > > + > > > + /* > > > * Whenever the number of active pipes changes, we need > > > to > > > make sure we > > > * update the pipes in the right order so that their > > > ddb > > > allocations > > > * never overlap with eachother inbetween CRTC updates. > > > Otherwise we'll > > > @@ -14261,6 +14308,9 @@ static void skl_update_crtcs(struct > > > drm_atomic_state *state, > > > progress = true; > > > } > > > } while (progress); > > > + > > > + if (!intel_state->wm_results.y_plane_mask) > > > + bxt_set_ipc_wa(dev_priv, false); > > > } > > > > > > static void intel_atomic_commit_tail(struct drm_atomic_state > > > *state) >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4737a0e..79b9305 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1632,6 +1632,8 @@ struct skl_wm_values { unsigned dirty_pipes; /* any WaterMark memory workaround Required */ enum watermark_memory_wa mem_wa; + /* IPC Y-tiled WA related member */ + u32 y_plane_mask; struct skl_ddb_allocation ddb; uint32_t wm_linetime[I915_MAX_PIPES]; uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8]; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 75b5b52..210d9b0 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5658,6 +5658,9 @@ enum { #define PLANE_NV12_BUF_CFG(pipe, plane) \ _MMIO_PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), _PLANE_NV12_BUF_CFG_2(pipe)) +#define CHICKEN_DCPR_1 _MMIO(0x46430) +#define IDLE_WAKEMEM_MASK (1 << 13) + /* SKL new cursor registers */ #define _CUR_BUF_CFG_A 0x7017c #define _CUR_BUF_CFG_B 0x7117c diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5f50f53..ee7f88e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12415,6 +12415,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, bool is_crtc_enabled = crtc_state->active; bool turn_off, turn_on, visible, was_visible; struct drm_framebuffer *fb = plane_state->fb; + struct intel_atomic_state *intel_state = + to_intel_atomic_state(plane_state->state); int ret; if (INTEL_GEN(dev) >= 9 && plane->type != DRM_PLANE_TYPE_CURSOR) { @@ -12501,6 +12503,15 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state, !needs_scaling(old_plane_state)) pipe_config->disable_lp_wm = true; + if (fb && (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED || + fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) { + intel_state->wm_results.y_plane_mask |= + (1 << drm_plane_index(plane)); + } else { + intel_state->wm_results.y_plane_mask &= + ~(1 << drm_plane_index(plane)); + } + return 0; } @@ -13963,6 +13974,10 @@ static int intel_atomic_check(struct drm_device *dev, if (ret) return ret; + /* Copy the Y-tile WA related states */ + intel_state->wm_results.y_plane_mask = + dev_priv->wm.skl_results.y_plane_mask; + for_each_crtc_in_state(state, crtc, crtc_state, i) { struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state); @@ -14204,6 +14219,32 @@ static void intel_update_crtcs(struct drm_atomic_state *state, } } +/* + * GEN9 IPC WA for Y-tiled + */ +void bxt_set_ipc_wa(struct drm_i915_private *dev_priv, bool enable) +{ + u32 val; + + if (!IS_BROXTON(dev_priv) || !i915.enable_ipc) + return; + + val = I915_READ(CHICKEN_DCPR_1); + /* + * If WA is already enabled or disabled + * no need to re-enable or disable it. + */ + if ((enable && (val & IDLE_WAKEMEM_MASK)) || + (!enable && !(val & IDLE_WAKEMEM_MASK))) + return; + + if (enable) + val |= IDLE_WAKEMEM_MASK; + else + val &= ~IDLE_WAKEMEM_MASK; + I915_WRITE(CHICKEN_DCPR_1, val); +} + static void skl_update_crtcs(struct drm_atomic_state *state, unsigned int *crtc_vblank_mask) { @@ -14219,6 +14260,12 @@ static void skl_update_crtcs(struct drm_atomic_state *state, enum pipe pipe; /* + * If IPC WA need to be enabled, enable it now + */ + if (intel_state->wm_results.y_plane_mask) + bxt_set_ipc_wa(dev_priv, true); + + /* * Whenever the number of active pipes changes, we need to make sure we * update the pipes in the right order so that their ddb allocations * never overlap with eachother inbetween CRTC updates. Otherwise we'll @@ -14261,6 +14308,9 @@ static void skl_update_crtcs(struct drm_atomic_state *state, progress = true; } } while (progress); + + if (!intel_state->wm_results.y_plane_mask) + bxt_set_ipc_wa(dev_priv, false); } static void intel_atomic_commit_tail(struct drm_atomic_state *state)