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

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 01-08-2024 18:40, Riana Tauro wrote:
> 
> 
> 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
AFAIK platforms below 12.70 doesn't have Stand alone Media. We can 
leverage (GRAPHICS_VERx100(xe) < 1270) check and gt->type field.

if(GRAPHICS_VERx100(xe) < 1270)
	gtidle->powergate_enable = RENDER_POWERGATE_ENABLE |
				   MEDIA_POWERGATE_ENABLE;
else if	(gt->type == GT_MEDIA)
	gtidle->powergate_enable = MEDIA_POWERGATE_ENABLE;
else
	gtidle->powergate_enable = RENDER_POWERGATE_ENABLE;

Regards,
Badal
> 
> 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 */