[Intel-xe] [PATCH] drm/xe: Do not disable rc6 in s2idle
Gupta, Anshuman
anshuman.gupta at intel.com
Mon Jun 5 04:36:06 UTC 2023
> -----Original Message-----
> From: Belgaumkar, Vinay <vinay.belgaumkar at intel.com>
> Sent: Monday, June 5, 2023 7:59 AM
> To: Tauro, Riana <riana.tauro at intel.com>; intel-xe at lists.freedesktop.org
> Cc: Gupta, Anshuman <anshuman.gupta at intel.com>; Dixit, Ashutosh
> <ashutosh.dixit at intel.com>; Nilawar, Badal <badal.nilawar at intel.com>
> Subject: Re: [PATCH] drm/xe: Do not disable rc6 in s2idle
>
>
> On 5/30/2023 11:59 PM, Riana Tauro wrote:
> > Currently rc6 is disabled for all suspend paths but it should be
> > disabled only for S3+.
>
> Hi Riana,
>
> Is this a requirement documented somewhere? If so, can we add a link to
> that?
RC6 is prerequisite to achieve s0ix on integrated, https://01.org/blogs/qwang59/2020/linux-s0ix-troubleshooting
On discrete it is pre-requisite to achieve L1 link state, and that again needed for s0ix on Intel Host.
Discrete requirement is not documented.
But irrespective of that I was pondering even why do we need to disable RC6 on S3 ?
Even that is not Documented.
Thanks,
Anshuman Gupta.
>
> >
> > add a function to check the target suspend state and disable
> > rc6 for suspend states above s2idle
> >
> > Signed-off-by: Riana Tauro <riana.tauro at intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_gt.c | 4 ++++
> > drivers/gpu/drm/xe/xe_guc.c | 5 +++++
> > drivers/gpu/drm/xe/xe_guc.h | 1 +
> > drivers/gpu/drm/xe/xe_guc_pc.c | 12 ++++++------
> > drivers/gpu/drm/xe/xe_guc_pc.h | 1 +
> > drivers/gpu/drm/xe/xe_pm.c | 14 ++++++++++++++
> > drivers/gpu/drm/xe/xe_pm.h | 2 ++
> > drivers/gpu/drm/xe/xe_uc.c | 5 +++++
> > drivers/gpu/drm/xe/xe_uc.h | 1 +
> > 9 files changed, 39 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> > index 18eda5b1377f..75c24d794436 100644
> > --- a/drivers/gpu/drm/xe/xe_gt.c
> > +++ b/drivers/gpu/drm/xe/xe_gt.c
> > @@ -32,6 +32,7 @@
> > #include "xe_migrate.h"
> > #include "xe_mmio.h"
> > #include "xe_pat.h"
> > +#include "xe_pm.h"
> > #include "xe_mocs.h"
> > #include "xe_reg_sr.h"
> > #include "xe_ring_ops.h"
> > @@ -613,6 +614,7 @@ static int gt_reset(struct xe_gt *gt)
> > if (err)
> > goto err_msg;
> >
> > + xe_uc_gucrc_disable(>->uc);
> > xe_uc_stop_prepare(>->uc);
> > xe_gt_pagefault_reset(gt);
> > xe_gt_tlb_invalidation_reset(gt);
> > @@ -671,6 +673,8 @@ void xe_gt_suspend_prepare(struct xe_gt *gt)
> > xe_device_mem_access_get(gt_to_xe(gt));
> > XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt),
> XE_FORCEWAKE_ALL));
> >
> > + if (xe_pm_suspend_target() > PM_SUSPEND_TO_IDLE)
> > + xe_uc_gucrc_disable(>->uc);
> > xe_uc_stop_prepare(>->uc);
> >
> > xe_device_mem_access_put(gt_to_xe(gt));
> > diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> > index ecc843d91f62..b40a84ed6c0b 100644
> > --- a/drivers/gpu/drm/xe/xe_guc.c
> > +++ b/drivers/gpu/drm/xe/xe_guc.c
> > @@ -777,6 +777,11 @@ void xe_guc_reset_wait(struct xe_guc *guc)
> > xe_guc_submit_reset_wait(guc);
> > }
> >
> > +void xe_guc_gucrc_disable(struct xe_guc *guc) {
> > + XE_WARN_ON(xe_guc_pc_gucrc_disable(&guc->pc));
> > +}
> > +
> > void xe_guc_stop_prepare(struct xe_guc *guc)
> > {
> > XE_WARN_ON(xe_guc_pc_stop(&guc->pc));
> > diff --git a/drivers/gpu/drm/xe/xe_guc.h b/drivers/gpu/drm/xe/xe_guc.h
> > index 74a74051f354..29449f456df4 100644
> > --- a/drivers/gpu/drm/xe/xe_guc.h
> > +++ b/drivers/gpu/drm/xe/xe_guc.h
> > @@ -35,6 +35,7 @@ void xe_guc_reset_wait(struct xe_guc *guc);
> > void xe_guc_stop_prepare(struct xe_guc *guc);
> > int xe_guc_stop(struct xe_guc *guc);
> > int xe_guc_start(struct xe_guc *guc);
> > +void xe_guc_gucrc_disable(struct xe_guc *guc);
> >
> > static inline u16 xe_engine_class_to_guc_class(enum xe_engine_class
> class)
> > {
> > diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c
> > b/drivers/gpu/drm/xe/xe_guc_pc.c index e799faa1c6b8..de4c7b5e8658
> > 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> > @@ -730,7 +730,11 @@ static int pc_adjust_requested_freq(struct
> xe_guc_pc *pc)
> > return ret;
> > }
> >
> > -static int pc_gucrc_disable(struct xe_guc_pc *pc)
> > +/**
> > + * xe_guc_pc_gucrc_disable - disable gucrc
> > + * @pc: Xe_GuC_PC instance
> > + */
> > +int xe_guc_pc_gucrc_disable(struct xe_guc_pc *pc)
> > {
> > struct xe_gt *gt = pc_to_gt(pc);
> > int ret;
> > @@ -827,7 +831,7 @@ int xe_guc_pc_start(struct xe_guc_pc *pc)
> > goto out;
> >
> > if (xe->info.platform == XE_PVC) {
> > - pc_gucrc_disable(pc);
> > + xe_guc_pc_gucrc_disable(pc);
> > ret = 0;
> > goto out;
> > }
> > @@ -850,10 +854,6 @@ int xe_guc_pc_stop(struct xe_guc_pc *pc)
> >
> > xe_device_mem_access_get(pc_to_xe(pc));
> >
> > - ret = pc_gucrc_disable(pc);
>
> shouldn't we replace this with xe_guc_pc_gucrc_disable() ?
>
> Thanks,
>
> Vinay.
>
> > - if (ret)
> > - goto out;
> > -
> > mutex_lock(&pc->freq_lock);
> > pc->freq_ready = false;
> > mutex_unlock(&pc->freq_lock);
> > diff --git a/drivers/gpu/drm/xe/xe_guc_pc.h
> > b/drivers/gpu/drm/xe/xe_guc_pc.h index da29e4934868..797cda7ede6e
> > 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_pc.h
> > +++ b/drivers/gpu/drm/xe/xe_guc_pc.h
> > @@ -11,5 +11,6 @@
> > int xe_guc_pc_init(struct xe_guc_pc *pc);
> > int xe_guc_pc_start(struct xe_guc_pc *pc);
> > int xe_guc_pc_stop(struct xe_guc_pc *pc);
> > +int xe_guc_pc_gucrc_disable(struct xe_guc_pc *pc);
> >
> > #endif /* _XE_GUC_PC_H_ */
> > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> > index b7b57f10ba25..fd91463c23a6 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.c
> > +++ b/drivers/gpu/drm/xe/xe_pm.c
> > @@ -199,6 +199,20 @@ int xe_pm_runtime_resume(struct xe_device *xe)
> > return 0;
> > }
> >
> > +/**
> > + * xe_pm_suspend_target - gets the current suspend target
> > + *
> > + * Return: suspend target if config_suspend is enabled, s2idle
> > +otherwise */ suspend_state_t xe_pm_suspend_target(void) { #if
> > +IS_ENABLED(CONFIG_SUSPEND)
> > + return pm_suspend_target_state;
> > +#else
> > + return PM_SUSPEND_TO_IDLE;
> > +#endif
> > +}
> > +
> > int xe_pm_runtime_get(struct xe_device *xe)
> > {
> > return pm_runtime_get_sync(xe->drm.dev); diff --git
> > a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h index
> > 6a885585f653..17087a69140f 100644
> > --- a/drivers/gpu/drm/xe/xe_pm.h
> > +++ b/drivers/gpu/drm/xe/xe_pm.h
> > @@ -7,6 +7,7 @@
> > #define _XE_PM_H_
> >
> > #include <linux/pm_runtime.h>
> > +#include <linux/suspend.h>
> >
> > struct xe_device;
> >
> > @@ -22,4 +23,5 @@ int xe_pm_runtime_put(struct xe_device *xe);
> > bool xe_pm_runtime_resume_if_suspended(struct xe_device *xe);
> > int xe_pm_runtime_get_if_active(struct xe_device *xe);
> >
> > +suspend_state_t xe_pm_suspend_target(void);
> > #endif
> > diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
> > index 70eabf567156..67c0e10f9f6d 100644
> > --- a/drivers/gpu/drm/xe/xe_uc.c
> > +++ b/drivers/gpu/drm/xe/xe_uc.c
> > @@ -176,6 +176,11 @@ int xe_uc_reset_prepare(struct xe_uc *uc)
> > return xe_guc_reset_prepare(&uc->guc);
> > }
> >
> > +void xe_uc_gucrc_disable(struct xe_uc *uc) {
> > + xe_guc_gucrc_disable(&uc->guc);
> > +}
> > +
> > void xe_uc_stop_prepare(struct xe_uc *uc)
> > {
> > xe_guc_stop_prepare(&uc->guc);
> > diff --git a/drivers/gpu/drm/xe/xe_uc.h b/drivers/gpu/drm/xe/xe_uc.h
> > index d6efc9ef00d3..b1c56c462d48 100644
> > --- a/drivers/gpu/drm/xe/xe_uc.h
> > +++ b/drivers/gpu/drm/xe/xe_uc.h
> > @@ -14,6 +14,7 @@ int xe_uc_init_post_hwconfig(struct xe_uc *uc);
> > int xe_uc_init_hw(struct xe_uc *uc);
> > int xe_uc_reset_prepare(struct xe_uc *uc);
> > void xe_uc_stop_prepare(struct xe_uc *uc);
> > +void xe_uc_gucrc_disable(struct xe_uc *uc);
> > int xe_uc_stop(struct xe_uc *uc);
> > int xe_uc_start(struct xe_uc *uc);
> > int xe_uc_suspend(struct xe_uc *uc);
More information about the Intel-xe
mailing list