diff mbox

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

Message ID 1409830075-11139-57-git-send-email-damien.lespiau@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lespiau, Damien Sept. 4, 2014, 11:27 a.m. UTC 
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(-)

Comments

Mika Kuoppala Sept. 24, 2014, 1:36 p.m. UTC | #1 
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.

> +	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

> +};
> +
> +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
Bob Wang Sept. 24, 2014, 6:16 p.m. UTC | #2 
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.
>> +};
>> +
>> +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
Mika Kuoppala Sept. 25, 2014, 8:58 a.m. UTC | #3 
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
diff mbox

Patch

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,
+	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 */
+};
+
+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; \