diff mbox

[v6,4/7] drm/i915/perf: reuse intel_lrc ctx regs macro

Message ID 20180522180002.11522-5-lionel.g.landwerlin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lionel Landwerlin May 22, 2018, 5:59 p.m. UTC 
Abstract the context image access a bit.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 34 +++++++++++++++-----------------
 1 file changed, 16 insertions(+), 18 deletions(-)

Comments

Tvrtko Ursulin May 23, 2018, 2:57 p.m. UTC | #1 
On 22/05/2018 18:59, Lionel Landwerlin wrote:
> Abstract the context image access a bit.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_perf.c | 34 +++++++++++++++-----------------
>   1 file changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 805dfc732bba..a5d98bda5c2e 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -210,6 +210,7 @@
>   #include "i915_oa_cflgt3.h"
>   #include "i915_oa_cnl.h"
>   #include "i915_oa_icl.h"
> +#include "intel_lrc_reg.h"
>   
>   /* HW requires this to be a power of two, between 128k and 16M, though driver
>    * is currently generally designed assuming the largest 16M size is used such
> @@ -1579,27 +1580,25 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
>   	u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
>   	u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
>   	/* The MMIO offsets for Flex EU registers aren't contiguous */
> -	u32 flex_mmio[] = {
> -		i915_mmio_reg_offset(EU_PERF_CNTL0),
> -		i915_mmio_reg_offset(EU_PERF_CNTL1),
> -		i915_mmio_reg_offset(EU_PERF_CNTL2),
> -		i915_mmio_reg_offset(EU_PERF_CNTL3),
> -		i915_mmio_reg_offset(EU_PERF_CNTL4),
> -		i915_mmio_reg_offset(EU_PERF_CNTL5),
> -		i915_mmio_reg_offset(EU_PERF_CNTL6),
> +	i915_reg_t flex_regs[] = {
> +		EU_PERF_CNTL0,
> +		EU_PERF_CNTL1,
> +		EU_PERF_CNTL2,
> +		EU_PERF_CNTL3,
> +		EU_PERF_CNTL4,
> +		EU_PERF_CNTL5,
> +		EU_PERF_CNTL6,
>   	};
>   	int i;
>   
> -	reg_state[ctx_oactxctrl] = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
> -	reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
> -				      GEN8_OA_TIMER_PERIOD_SHIFT) |
> -				     (dev_priv->perf.oa.periodic ?
> -				      GEN8_OA_TIMER_ENABLE : 0) |
> -				     GEN8_OA_COUNTER_RESUME;
> +	CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
> +		(dev_priv->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
> +		(dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
> +		GEN8_OA_COUNTER_RESUME);
>   
> -	for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
> +	for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
>   		u32 state_offset = ctx_flexeu0 + i * 2;
> -		u32 mmio = flex_mmio[i];
> +		u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
>   
>   		/*
>   		 * This arbitrary default will select the 'EU FPU0 Pipeline
> @@ -1619,8 +1618,7 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
>   			}
>   		}
>   
> -		reg_state[state_offset] = mmio;
> -		reg_state[state_offset+1] = value;
> +		CTX_REG(reg_state, state_offset, flex_regs[i], value);

Does flex_regs[i] needs to be mmio?

>   	}
>   }
>   
> 

Regards,

Tvrtko
Lionel Landwerlin May 23, 2018, 3:54 p.m. UTC | #2 
On 23/05/18 15:57, Tvrtko Ursulin wrote:
>
> On 22/05/2018 18:59, Lionel Landwerlin wrote:
>> Abstract the context image access a bit.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c | 34 +++++++++++++++-----------------
>>   1 file changed, 16 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
>> b/drivers/gpu/drm/i915/i915_perf.c
>> index 805dfc732bba..a5d98bda5c2e 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -210,6 +210,7 @@
>>   #include "i915_oa_cflgt3.h"
>>   #include "i915_oa_cnl.h"
>>   #include "i915_oa_icl.h"
>> +#include "intel_lrc_reg.h"
>>     /* HW requires this to be a power of two, between 128k and 16M, 
>> though driver
>>    * is currently generally designed assuming the largest 16M size is 
>> used such
>> @@ -1579,27 +1580,25 @@ static void 
>> gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
>>       u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
>>       u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
>>       /* The MMIO offsets for Flex EU registers aren't contiguous */
>> -    u32 flex_mmio[] = {
>> -        i915_mmio_reg_offset(EU_PERF_CNTL0),
>> -        i915_mmio_reg_offset(EU_PERF_CNTL1),
>> -        i915_mmio_reg_offset(EU_PERF_CNTL2),
>> -        i915_mmio_reg_offset(EU_PERF_CNTL3),
>> -        i915_mmio_reg_offset(EU_PERF_CNTL4),
>> -        i915_mmio_reg_offset(EU_PERF_CNTL5),
>> -        i915_mmio_reg_offset(EU_PERF_CNTL6),
>> +    i915_reg_t flex_regs[] = {
>> +        EU_PERF_CNTL0,
>> +        EU_PERF_CNTL1,
>> +        EU_PERF_CNTL2,
>> +        EU_PERF_CNTL3,
>> +        EU_PERF_CNTL4,
>> +        EU_PERF_CNTL5,
>> +        EU_PERF_CNTL6,
>>       };
>>       int i;
>>   -    reg_state[ctx_oactxctrl] = 
>> i915_mmio_reg_offset(GEN8_OACTXCONTROL);
>> -    reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
>> -                      GEN8_OA_TIMER_PERIOD_SHIFT) |
>> -                     (dev_priv->perf.oa.periodic ?
>> -                      GEN8_OA_TIMER_ENABLE : 0) |
>> -                     GEN8_OA_COUNTER_RESUME;
>> +    CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
>> +        (dev_priv->perf.oa.period_exponent << 
>> GEN8_OA_TIMER_PERIOD_SHIFT) |
>> +        (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
>> +        GEN8_OA_COUNTER_RESUME);
>>   -    for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
>> +    for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
>>           u32 state_offset = ctx_flexeu0 + i * 2;
>> -        u32 mmio = flex_mmio[i];
>> +        u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
>>             /*
>>            * This arbitrary default will select the 'EU FPU0 Pipeline
>> @@ -1619,8 +1618,7 @@ static void 
>> gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
>>               }
>>           }
>>   -        reg_state[state_offset] = mmio;
>> -        reg_state[state_offset+1] = value;
>> +        CTX_REG(reg_state, state_offset, flex_regs[i], value);
>
> Does flex_regs[i] needs to be mmio?

Yeah, the macro uses i915_mmio_reg_offset().

>
>>       }
>>   }
>>
>
> Regards,
>
> Tvrtko
>
Tvrtko Ursulin May 24, 2018, 10:33 a.m. UTC | #3 
On 23/05/2018 16:54, Lionel Landwerlin wrote:
> On 23/05/18 15:57, Tvrtko Ursulin wrote:
>>
>> On 22/05/2018 18:59, Lionel Landwerlin wrote:
>>> Abstract the context image access a bit.
>>>
>>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_perf.c | 34 +++++++++++++++-----------------
>>>   1 file changed, 16 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
>>> b/drivers/gpu/drm/i915/i915_perf.c
>>> index 805dfc732bba..a5d98bda5c2e 100644
>>> --- a/drivers/gpu/drm/i915/i915_perf.c
>>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>>> @@ -210,6 +210,7 @@
>>>   #include "i915_oa_cflgt3.h"
>>>   #include "i915_oa_cnl.h"
>>>   #include "i915_oa_icl.h"
>>> +#include "intel_lrc_reg.h"
>>>     /* HW requires this to be a power of two, between 128k and 16M, 
>>> though driver
>>>    * is currently generally designed assuming the largest 16M size is 
>>> used such
>>> @@ -1579,27 +1580,25 @@ static void 
>>> gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
>>>       u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
>>>       u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
>>>       /* The MMIO offsets for Flex EU registers aren't contiguous */
>>> -    u32 flex_mmio[] = {
>>> -        i915_mmio_reg_offset(EU_PERF_CNTL0),
>>> -        i915_mmio_reg_offset(EU_PERF_CNTL1),
>>> -        i915_mmio_reg_offset(EU_PERF_CNTL2),
>>> -        i915_mmio_reg_offset(EU_PERF_CNTL3),
>>> -        i915_mmio_reg_offset(EU_PERF_CNTL4),
>>> -        i915_mmio_reg_offset(EU_PERF_CNTL5),
>>> -        i915_mmio_reg_offset(EU_PERF_CNTL6),
>>> +    i915_reg_t flex_regs[] = {
>>> +        EU_PERF_CNTL0,
>>> +        EU_PERF_CNTL1,
>>> +        EU_PERF_CNTL2,
>>> +        EU_PERF_CNTL3,
>>> +        EU_PERF_CNTL4,
>>> +        EU_PERF_CNTL5,
>>> +        EU_PERF_CNTL6,
>>>       };
>>>       int i;
>>>   -    reg_state[ctx_oactxctrl] = 
>>> i915_mmio_reg_offset(GEN8_OACTXCONTROL);
>>> -    reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
>>> -                      GEN8_OA_TIMER_PERIOD_SHIFT) |
>>> -                     (dev_priv->perf.oa.periodic ?
>>> -                      GEN8_OA_TIMER_ENABLE : 0) |
>>> -                     GEN8_OA_COUNTER_RESUME;
>>> +    CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
>>> +        (dev_priv->perf.oa.period_exponent << 
>>> GEN8_OA_TIMER_PERIOD_SHIFT) |
>>> +        (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
>>> +        GEN8_OA_COUNTER_RESUME);
>>>   -    for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
>>> +    for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
>>>           u32 state_offset = ctx_flexeu0 + i * 2;
>>> -        u32 mmio = flex_mmio[i];
>>> +        u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
>>>             /*
>>>            * This arbitrary default will select the 'EU FPU0 Pipeline
>>> @@ -1619,8 +1618,7 @@ static void 
>>> gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
>>>               }
>>>           }
>>>   -        reg_state[state_offset] = mmio;
>>> -        reg_state[state_offset+1] = value;
>>> +        CTX_REG(reg_state, state_offset, flex_regs[i], value);
>>
>> Does flex_regs[i] needs to be mmio?
> 
> Yeah, the macro uses i915_mmio_reg_offset().

In that case:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 805dfc732bba..a5d98bda5c2e 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -210,6 +210,7 @@ 
 #include "i915_oa_cflgt3.h"
 #include "i915_oa_cnl.h"
 #include "i915_oa_icl.h"
+#include "intel_lrc_reg.h"
 
 /* HW requires this to be a power of two, between 128k and 16M, though driver
  * is currently generally designed assuming the largest 16M size is used such
@@ -1579,27 +1580,25 @@  static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
 	u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
 	u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
 	/* The MMIO offsets for Flex EU registers aren't contiguous */
-	u32 flex_mmio[] = {
-		i915_mmio_reg_offset(EU_PERF_CNTL0),
-		i915_mmio_reg_offset(EU_PERF_CNTL1),
-		i915_mmio_reg_offset(EU_PERF_CNTL2),
-		i915_mmio_reg_offset(EU_PERF_CNTL3),
-		i915_mmio_reg_offset(EU_PERF_CNTL4),
-		i915_mmio_reg_offset(EU_PERF_CNTL5),
-		i915_mmio_reg_offset(EU_PERF_CNTL6),
+	i915_reg_t flex_regs[] = {
+		EU_PERF_CNTL0,
+		EU_PERF_CNTL1,
+		EU_PERF_CNTL2,
+		EU_PERF_CNTL3,
+		EU_PERF_CNTL4,
+		EU_PERF_CNTL5,
+		EU_PERF_CNTL6,
 	};
 	int i;
 
-	reg_state[ctx_oactxctrl] = i915_mmio_reg_offset(GEN8_OACTXCONTROL);
-	reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent <<
-				      GEN8_OA_TIMER_PERIOD_SHIFT) |
-				     (dev_priv->perf.oa.periodic ?
-				      GEN8_OA_TIMER_ENABLE : 0) |
-				     GEN8_OA_COUNTER_RESUME;
+	CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
+		(dev_priv->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
+		(dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
+		GEN8_OA_COUNTER_RESUME);
 
-	for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) {
+	for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
 		u32 state_offset = ctx_flexeu0 + i * 2;
-		u32 mmio = flex_mmio[i];
+		u32 mmio = i915_mmio_reg_offset(flex_regs[i]);
 
 		/*
 		 * This arbitrary default will select the 'EU FPU0 Pipeline
@@ -1619,8 +1618,7 @@  static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
 			}
 		}
 
-		reg_state[state_offset] = mmio;
-		reg_state[state_offset+1] = value;
+		CTX_REG(reg_state, state_offset, flex_regs[i], value);
 	}
 }