[Intel-gfx] [PATCH v6 2/8] drm/i915/bxt: IPC WA for Broxton

[Mahesh Kumar]

Hi,

Will keep WA number in commit message/WA location.
thanks,

Regards,

-Mahesh

On Thursday 24 November 2016 06:21 PM, Lankhorst, Maarten wrote:
> Mahesh Kumar schreef op do 24-11-2016 om 09:31 [+0530]:
>> If IPC is enabled in BXT, display underruns are observed.
>> WA: The Line Time programmed in the WM_LINETIME register should be
>> half of the actual calculated Line Time.
>>
>> Programmed Line Time = 1/2*Calculated Line Time
>>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.c |  2 ++
>>   drivers/gpu/drm/i915/i915_drv.h |  2 ++
>>   drivers/gpu/drm/i915/intel_pm.c | 11 +++++++++--
>>   3 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 445fec9..1b0a589 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1243,6 +1243,8 @@ int i915_driver_load(struct pci_dev *pdev,
>> const struct pci_device_id *ent)
>>   
>>   	intel_runtime_pm_enable(dev_priv);
>>   
>> +	dev_priv->ipc_enabled = false;
>> +
>>   	/* Everything is in place, we can now relax! */
>>   	DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>>   		 driver.name, driver.major, driver.minor,
>> driver.patchlevel,
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 63c0ea0..6691a4e 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2128,6 +2128,8 @@ struct drm_i915_private {
>>   	/* perform PHY state sanity checks? */
>>   	bool chv_phy_assert[2];
>>   
>> +	bool ipc_enabled;
>> +
>>   	/* Used to save the pipe-to-encoder mapping for audio */
>>   	struct intel_encoder *av_enc_map[I915_MAX_PIPES];
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
>> b/drivers/gpu/drm/i915/intel_pm.c
>> index 8908736..7090a7c 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3769,7 +3769,10 @@ skl_compute_wm_level(const struct
>> drm_i915_private *dev_priv,
>>   static uint32_t
>>   skl_compute_linetime_wm(struct intel_crtc_state *cstate)
>>   {
>> +	struct drm_atomic_state *state = cstate->base.state;
>> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
>>   	uint32_t pixel_rate;
>> +	uint32_t linetime_wm;
>>   
>>   	if (!cstate->base.active)
>>   		return 0;
>> @@ -3779,8 +3782,12 @@ skl_compute_linetime_wm(struct
>> intel_crtc_state *cstate)
>>   	if (WARN_ON(pixel_rate == 0))
>>   		return 0;
>>   
>> -	return DIV_ROUND_UP(8 * cstate-
>>> base.adjusted_mode.crtc_htotal * 1000,
>> -			    pixel_rate);
>> +	linetime_wm = DIV_ROUND_UP(8 * cstate-
>>> base.adjusted_mode.crtc_htotal *
>> +						1000, pixel_rate);
>> +	if (IS_BROXTON(dev_priv) && dev_priv->ipc_enabled)
>> +		linetime_wm = DIV_ROUND_UP(linetime_wm, 2);
> I've asked on irc to be sure, but this needs slightly more info in the
> code as well. Same for 3/8 and 8/8.
>
> 12:47 < mlankhorst> danvet: if we add hw workarounds that don't have a
> name, do we still have to write comments about them in the code?
> 12:48 < danvet> at least a bspec reference
> 12:48 < danvet> some hint to make it possible to find where it's from
> 12:49 < danvet> e.g. mesa has traditionally just pasted the entire
> bspec paragraph
> 12:49 < mlankhorst> ok
> 12:49 < danvet> we've done that too in the past, but then the wa db
> mostly made that redundant
> 12:49 < danvet> but if there's no wa entry, then full deal
> 12:49 < danvet> also, maybe a bspec bug notice to pls add one
> 12:49 < danvet> you should be able to do that over the web, and afaik
> there should be one for every w/a