[Intel-gfx] [PATCH v3 8/9] drm/i915/bxt: set chicken bit as IPC y-tile WA
Mahesh Kumar
mahesh1.kumar at intel.com
Thu Sep 22 09:43:00 UTC 2016
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 at 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 at 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)
More information about the Intel-gfx
mailing list