Re: [PATCH 56/89] drm/i915: Gen9 shadowed registers

Mika Kuoppala
2014-09-25T08:58:21Z

Bob Wang <zhe1.wang@intel.com> writes:

> On 09/24/2014 02:36 PM, Mika Kuoppala wrote:
>> Damien Lespiau <damien.lespiau@intel.com> writes:
>>
>>> From: Zhe Wang <zhe1.wang@intel.com>
>>>
>>> For MMIO registers which are shadowed, force wake is not needed to
>>> write to these registers.
>>>
>>> v2: Rebase on top of nightly (Damien)
>>>
>>> Signed-off-by: Zhe Wang <zhe1.wang@intel.com>
>>> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_uncore.c | 26 +++++++++++++++++++++++++-
>>> 1 file changed, 25 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>>> index f289f4f..ce1214b 100644
>>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>>> @@ -1028,13 +1028,37 @@ chv_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
>>> REG_WRITE_FOOTER; \
>>> }
>>>
>>> +static const u32 gen9_shadowed_regs[] = {
>>> + FORCEWAKE_MEDIA_GEN9,
>>> + FORCEWAKE_RENDER_GEN9,
>>> + FORCEWAKE_BLITTER_GEN9,
>> You might want to move the ring tails first on the list
>> and forcewake regs to bottom, to reflect the popularity
>> of access.
> good suggestion. a better order I can think of. comments welcomed.
>
> RING_TAIL(RENDER_RING_BASE),
> RING_TAIL(GEN6_BSD_RING_BASE),
> RING_TAIL(VEBOX_RING_BASE),
> RING_TAIL(BLT_RING_BASE),
> FORCEWAKE_BLITTER_GEN9,
> FORCEWAKE_RENDER_GEN9,
> FORCEWAKE_MEDIA_GEN9,
> GEN6_RPNSWREQ,
> GEN6_RC_VIDEO_FREQ,
>
>
>>> + GEN6_RPNSWREQ,
>>> + GEN6_RC_VIDEO_FREQ,
>>> + RING_TAIL(RENDER_RING_BASE),
>>> + RING_TAIL(GEN6_BSD_RING_BASE),
>>> + RING_TAIL(VEBOX_RING_BASE),
>>> + RING_TAIL(BLT_RING_BASE),
>>> + /* TODO: Other registers are not yet used */
>> I think we need exec list ones here also, but couldn't find
>> certain proof. Atleast with bdw we are missing them.
>>
>> -Mika
> Good catch. Execlist ones are on the list. We need to add those since
> execlist is enabled.

On closer look, it seems that the exec lists do their own forcewake
handling.

-Mika

>>> +};
>>> +
>>> +static bool is_gen9_shadowed(struct drm_i915_private *dev_priv, u32 reg)
>>> +{
>>> + int i;
>>> + for (i = 0; i < ARRAY_SIZE(gen9_shadowed_regs); i++)
>>> + if (reg == gen9_shadowed_regs[i])
>>> + return true;
>>> +
>>> + return false;
>>> +}
>>> +
>>> #define __gen9_write(x) \
>>> static void \
>>> gen9_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, \
>>> bool trace) { \
>>> REG_WRITE_HEADER; \
>>> if (!NEEDS_FORCE_WAKE((dev_priv), (reg)) || \
>>> - FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg)) { \
>>> + FORCEWAKE_GEN9_UNCORE_RANGE_OFFSET(reg) || \
>>> + is_gen9_shadowed(dev_priv, reg)) { \
>>> __raw_i915_write##x(dev_priv, reg, val); \
>>> } else { \
>>> unsigned fwengine = 0; \
>>> --
>>> 1.8.3.1
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx