[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