Re: [PATCH v3 1/2] drm/xe/xe_gt_idle: store powergate enable bits in gtidle

[Riana Tauro <riana.tauro@intel.com>]

On 8/1/2024 3:37 PM, Nilawar, Badal wrote:
> 
> 
> On 01-08-2024 15:23, Riana Tauro wrote:
>> Have a copy of the value written to powergate enable
>> register. No functional changes
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_gt_idle.c       | 18 ++++++++++--------
>>   drivers/gpu/drm/xe/xe_gt_idle_types.h |  2 ++
>>   2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_idle.c 
>> b/drivers/gpu/drm/xe/xe_gt_idle.c
>> index 67aba4140510..7188542aea43 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_idle.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_idle.c
>> @@ -98,7 +98,7 @@ static u64 get_residency_ms(struct xe_gt_idle 
>> *gtidle, u64 cur_residency)
>>   void xe_gt_idle_enable_pg(struct xe_gt *gt)
>>   {
>>       struct xe_device *xe = gt_to_xe(gt);
>> -    u32 pg_enable;
>> +    struct xe_gt_idle *gtidle = &gt->gtidle;
>>       int i, j;
>>       if (IS_SRIOV_VF(xe))
>> @@ -110,12 +110,12 @@ void xe_gt_idle_enable_pg(struct xe_gt *gt)
>>       xe_device_assert_mem_access(gt_to_xe(gt));
>> -    pg_enable = RENDER_POWERGATE_ENABLE | MEDIA_POWERGATE_ENABLE;
>> +    gtidle->powergate_enable = RENDER_POWERGATE_ENABLE | 
>> MEDIA_POWERGATE_ENABLE;
> Here I think we should have platform check. Like on MTL, LNL RPG 
> applicable for only for GT (GT0) and Media PG is applicable only for 
> Media GT (GT1) ?

We cannot have platform check for this as its applicable even for 
products that do not have a separate media gt

Thanks,
Riana
> 
> Regards,
> Badal
>>       for (i = XE_HW_ENGINE_VCS0, j = 0; i <= XE_HW_ENGINE_VCS7; ++i, 
>> ++j) {
>>           if ((gt->info.engine_mask & BIT(i)))
>> -            pg_enable |= (VDN_HCP_POWERGATE_ENABLE(j) |
>> -                      VDN_MFXVDENC_POWERGATE_ENABLE(j));
>> +            gtidle->powergate_enable |= (VDN_HCP_POWERGATE_ENABLE(j) |
>> +                             VDN_MFXVDENC_POWERGATE_ENABLE(j));
>>       }
>>       XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT));
>> @@ -128,20 +128,22 @@ void xe_gt_idle_enable_pg(struct xe_gt *gt)
>>           xe_mmio_write32(gt, RENDER_POWERGATE_IDLE_HYSTERESIS, 25);
>>       }
>> -    xe_mmio_write32(gt, POWERGATE_ENABLE, pg_enable);
>> +    xe_mmio_write32(gt, POWERGATE_ENABLE, gtidle->powergate_enable);
>>       XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
>>   }
>>   void xe_gt_idle_disable_pg(struct xe_gt *gt)
>>   {
>> +    struct xe_gt_idle *gtidle = &gt->gtidle;
>> +
>>       if (IS_SRIOV_VF(gt_to_xe(gt)))
>>           return;
>>       xe_device_assert_mem_access(gt_to_xe(gt));
>> -    XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT));
>> -
>> -    xe_mmio_write32(gt, POWERGATE_ENABLE, 0);
>> +    gtidle->powergate_enable = 0;
>> +    XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT));
>> +    xe_mmio_write32(gt, POWERGATE_ENABLE, gtidle->powergate_enable);
>>       XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT));
>>   }
>> diff --git a/drivers/gpu/drm/xe/xe_gt_idle_types.h 
>> b/drivers/gpu/drm/xe/xe_gt_idle_types.h
>> index f99b447534f3..7a8e63f2ebcc 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_idle_types.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_idle_types.h
>> @@ -23,6 +23,8 @@ enum xe_gt_idle_state {
>>   struct xe_gt_idle {
>>       /** @name: name */
>>       char name[16];
>> +    /** powergate_enable: copy of powergate enable bits*/
>> +    u32 powergate_enable;
>>       /** @residency_multiplier: residency multiplier in ns */
>>       u32 residency_multiplier;
>>       /** @cur_residency: raw driver copy of idle residency */