* [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend
@ 2024-08-06 8:55 Arun R Murthy
2024-08-06 9:11 ` ✓ CI.Patch_applied: success for " Patchwork
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Arun R Murthy @ 2024-08-06 8:55 UTC (permalink / raw)
To: intel-xe, intel-gfx; +Cc: Arun R Murthy
HPD is interrupt based and on runtime suspend change it to polling as
HPD is not a wakeup event. A worker thread is scheduled for doing this
polling and it keeps polling for HPD live status on an internval of 10s.
On runtime resume disable polling and fallback to interrupt mechanism.
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
drivers/gpu/drm/xe/xe_pm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
index 9f3c14fd9f33..d952b06ebfb4 100644
--- a/drivers/gpu/drm/xe/xe_pm.c
+++ b/drivers/gpu/drm/xe/xe_pm.c
@@ -22,6 +22,7 @@
#include "xe_pcode.h"
#include "xe_trace.h"
#include "xe_wa.h"
+#include "intel_hotplug.h"
/**
* DOC: Xe Power Management
@@ -378,6 +379,8 @@ int xe_pm_runtime_suspend(struct xe_device *xe)
if (err)
goto out;
}
+ /* Enable hpd polling on runtime suspend */
+ intel_hpd_poll_enable(xe);
xe_irq_suspend(xe);
@@ -427,6 +430,9 @@ int xe_pm_runtime_resume(struct xe_device *xe)
xe_irq_resume(xe);
+ /* Disable hpd polling on runtime resume */
+ intel_hpd_poll_disable(xe);
+
for_each_gt(gt, xe, id)
xe_gt_resume(gt);
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* ✓ CI.Patch_applied: success for drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy @ 2024-08-06 9:11 ` Patchwork 2024-08-06 9:11 ` ✓ CI.checkpatch: " Patchwork ` (5 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2024-08-06 9:11 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-xe == Series Details == Series: drm/xe/pm: Change HPD to polling on runtime suspend URL : https://patchwork.freedesktop.org/series/136916/ State : success == Summary == === Applying kernel patches on branch 'drm-tip' with base: === Base commit: 28edc41db567 drm-tip: 2024y-08m-05d-21h-28m-29s UTC integration manifest === git am output follows === Applying: drm/xe/pm: Change HPD to polling on runtime suspend ^ permalink raw reply [flat|nested] 11+ messages in thread
* ✓ CI.checkpatch: success for drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy 2024-08-06 9:11 ` ✓ CI.Patch_applied: success for " Patchwork @ 2024-08-06 9:11 ` Patchwork 2024-08-06 9:12 ` ✗ CI.KUnit: failure " Patchwork ` (4 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2024-08-06 9:11 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-xe == Series Details == Series: drm/xe/pm: Change HPD to polling on runtime suspend URL : https://patchwork.freedesktop.org/series/136916/ State : success == Summary == + KERNEL=/kernel + git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt Cloning into 'mt'... warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/ + git -C mt rev-list -n1 origin/master 66551fd1be58212fb46d0bc97f2e5ae43c69b733 + cd /kernel + git config --global --add safe.directory /kernel + git log -n1 commit a93b19e1fe9b5150ed1a5361e3733cc4a33b2499 Author: Arun R Murthy <arun.r.murthy@intel.com> Date: Tue Aug 6 14:25:13 2024 +0530 drm/xe/pm: Change HPD to polling on runtime suspend HPD is interrupt based and on runtime suspend change it to polling as HPD is not a wakeup event. A worker thread is scheduled for doing this polling and it keeps polling for HPD live status on an internval of 10s. On runtime resume disable polling and fallback to interrupt mechanism. Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> + /mt/dim checkpatch 28edc41db567c4c555fff23119c10d39689131c2 drm-intel a93b19e1fe9b drm/xe/pm: Change HPD to polling on runtime suspend ^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ CI.KUnit: failure for drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy 2024-08-06 9:11 ` ✓ CI.Patch_applied: success for " Patchwork 2024-08-06 9:11 ` ✓ CI.checkpatch: " Patchwork @ 2024-08-06 9:12 ` Patchwork 2024-08-06 16:17 ` [PATCH] " kernel test robot ` (3 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2024-08-06 9:12 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-xe == Series Details == Series: drm/xe/pm: Change HPD to polling on runtime suspend URL : https://patchwork.freedesktop.org/series/136916/ State : failure == Summary == + trap cleanup EXIT + /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig ERROR:root:../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes] 156 | u64 ioread64_lo_hi(const void __iomem *addr) | ^~~~~~~~~~~~~~ ../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes] 163 | u64 ioread64_hi_lo(const void __iomem *addr) | ^~~~~~~~~~~~~~ ../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes] 170 | u64 ioread64be_lo_hi(const void __iomem *addr) | ^~~~~~~~~~~~~~~~ ../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes] 178 | u64 ioread64be_hi_lo(const void __iomem *addr) | ^~~~~~~~~~~~~~~~ ../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes] 264 | void iowrite64_lo_hi(u64 val, void __iomem *addr) | ^~~~~~~~~~~~~~~ ../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes] 272 | void iowrite64_hi_lo(u64 val, void __iomem *addr) | ^~~~~~~~~~~~~~~ ../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes] 280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr) | ^~~~~~~~~~~~~~~~~ ../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes] 288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr) | ^~~~~~~~~~~~~~~~~ ../drivers/gpu/drm/xe/xe_pm.c:25:10: fatal error: intel_hotplug.h: No such file or directory 25 | #include "intel_hotplug.h" | ^~~~~~~~~~~~~~~~~ compilation terminated. make[7]: *** [../scripts/Makefile.build:244: drivers/gpu/drm/xe/xe_pm.o] Error 1 make[7]: *** Waiting for unfinished jobs.... make[6]: *** [../scripts/Makefile.build:485: drivers/gpu/drm/xe] Error 2 make[6]: *** Waiting for unfinished jobs.... make[5]: *** [../scripts/Makefile.build:485: drivers/gpu/drm] Error 2 make[4]: *** [../scripts/Makefile.build:485: drivers/gpu] Error 2 make[3]: *** [../scripts/Makefile.build:485: drivers] Error 2 make[2]: *** [/kernel/Makefile:1925: .] Error 2 make[1]: *** [/kernel/Makefile:224: __sub-make] Error 2 make: *** [Makefile:224: __sub-make] Error 2 [09:11:55] Configuring KUnit Kernel ... Generating .config ... Populating config with: $ make ARCH=um O=.kunit olddefconfig [09:11:59] Building KUnit Kernel ... Populating config with: $ make ARCH=um O=.kunit olddefconfig Building with: $ make ARCH=um O=.kunit --jobs=48 + cleanup ++ stat -c %u:%g /kernel + chown -R 1003:1003 /kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy ` (2 preceding siblings ...) 2024-08-06 9:12 ` ✗ CI.KUnit: failure " Patchwork @ 2024-08-06 16:17 ` kernel test robot 2024-08-06 16:41 ` Imre Deak ` (2 subsequent siblings) 6 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2024-08-06 16:17 UTC (permalink / raw) To: Arun R Murthy, intel-xe, intel-gfx; +Cc: oe-kbuild-all, Arun R Murthy Hi Arun, kernel test robot noticed the following build errors: [auto build test ERROR on drm-xe/drm-xe-next] [also build test ERROR on drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.11-rc2 next-20240806] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Arun-R-Murthy/drm-xe-pm-Change-HPD-to-polling-on-runtime-suspend/20240806-175155 base: https://gitlab.freedesktop.org/drm/xe/kernel.git drm-xe-next patch link: https://lore.kernel.org/r/20240806085513.3051107-1-arun.r.murthy%40intel.com patch subject: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend config: i386-buildonly-randconfig-003-20240806 (https://download.01.org/0day-ci/archive/20240806/202408062332.8Cl1FjhB-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240806/202408062332.8Cl1FjhB-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408062332.8Cl1FjhB-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/gpu/drm/xe/xe_pm.c:25:10: fatal error: intel_hotplug.h: No such file or directory 25 | #include "intel_hotplug.h" | ^~~~~~~~~~~~~~~~~ compilation terminated. vim +25 drivers/gpu/drm/xe/xe_pm.c 12 13 #include "display/xe_display.h" 14 #include "xe_bo.h" 15 #include "xe_bo_evict.h" 16 #include "xe_device.h" 17 #include "xe_device_sysfs.h" 18 #include "xe_ggtt.h" 19 #include "xe_gt.h" 20 #include "xe_guc.h" 21 #include "xe_irq.h" 22 #include "xe_pcode.h" 23 #include "xe_trace.h" 24 #include "xe_wa.h" > 25 #include "intel_hotplug.h" 26 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy ` (3 preceding siblings ...) 2024-08-06 16:17 ` [PATCH] " kernel test robot @ 2024-08-06 16:41 ` Imre Deak 2024-08-07 9:15 ` Murthy, Arun R 2024-08-08 5:56 ` [v2PATCH] drm/xe/display: " Arun R Murthy 2024-08-08 6:58 ` ✗ CI.Patch_applied: failure for drm/xe/pm: Change HPD to polling on runtime suspend (rev2) Patchwork 6 siblings, 1 reply; 11+ messages in thread From: Imre Deak @ 2024-08-06 16:41 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-xe, intel-gfx On Tue, Aug 06, 2024 at 02:25:13PM +0530, Arun R Murthy wrote: > HPD is interrupt based and on runtime suspend change it to polling as > HPD is not a wakeup event. A worker thread is scheduled for doing this > polling and it keeps polling for HPD live status on an internval of 10s. > On runtime resume disable polling and fallback to interrupt mechanism. > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > --- > drivers/gpu/drm/xe/xe_pm.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c > index 9f3c14fd9f33..d952b06ebfb4 100644 > --- a/drivers/gpu/drm/xe/xe_pm.c > +++ b/drivers/gpu/drm/xe/xe_pm.c > @@ -22,6 +22,7 @@ > #include "xe_pcode.h" > #include "xe_trace.h" > #include "xe_wa.h" > +#include "intel_hotplug.h" > > /** > * DOC: Xe Power Management > @@ -378,6 +379,8 @@ int xe_pm_runtime_suspend(struct xe_device *xe) > if (err) > goto out; > } > + /* Enable hpd polling on runtime suspend */ > + intel_hpd_poll_enable(xe); > > xe_irq_suspend(xe); > > @@ -427,6 +430,9 @@ int xe_pm_runtime_resume(struct xe_device *xe) > > xe_irq_resume(xe); > > + /* Disable hpd polling on runtime resume */ > + intel_hpd_poll_disable(xe); This should be handled in xe_display_pm_resume() (likewise enabling polling in xe_display_pm_suspend()), in fact it's already called there. Also this is missing the xe->info.enable_display check and polling shouldn't be enabled when system (vs. runtime) suspending. Not sure why the display is suspended/resumed only if d3cold is allowed, for many of the display s/r steps at least it doesn't make sense to me. For now moving the d3cold check within xe_display_pm_suspend()/resume() and enabling/disabling polling regardless of that flag would be the simplest imo. > + > for_each_gt(gt, xe, id) > xe_gt_resume(gt); > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-06 16:41 ` Imre Deak @ 2024-08-07 9:15 ` Murthy, Arun R 2024-08-07 14:14 ` Imre Deak 0 siblings, 1 reply; 11+ messages in thread From: Murthy, Arun R @ 2024-08-07 9:15 UTC (permalink / raw) To: Deak, Imre Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org > -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Tuesday, August 6, 2024 10:11 PM > To: Murthy, Arun R <arun.r.murthy@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend > > On Tue, Aug 06, 2024 at 02:25:13PM +0530, Arun R Murthy wrote: > > HPD is interrupt based and on runtime suspend change it to polling as > > HPD is not a wakeup event. A worker thread is scheduled for doing this > > polling and it keeps polling for HPD live status on an internval of 10s. > > On runtime resume disable polling and fallback to interrupt mechanism. > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > --- > > drivers/gpu/drm/xe/xe_pm.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c > > index 9f3c14fd9f33..d952b06ebfb4 100644 > > --- a/drivers/gpu/drm/xe/xe_pm.c > > +++ b/drivers/gpu/drm/xe/xe_pm.c > > @@ -22,6 +22,7 @@ > > #include "xe_pcode.h" > > #include "xe_trace.h" > > #include "xe_wa.h" > > +#include "intel_hotplug.h" > > > > /** > > * DOC: Xe Power Management > > @@ -378,6 +379,8 @@ int xe_pm_runtime_suspend(struct xe_device *xe) > > if (err) > > goto out; > > } > > + /* Enable hpd polling on runtime suspend */ > > + intel_hpd_poll_enable(xe); > > > > xe_irq_suspend(xe); > > > > @@ -427,6 +430,9 @@ int xe_pm_runtime_resume(struct xe_device *xe) > > > > xe_irq_resume(xe); > > > > + /* Disable hpd polling on runtime resume */ > > + intel_hpd_poll_disable(xe); > > This should be handled in xe_display_pm_resume() (likewise enabling polling in > xe_display_pm_suspend()), in fact it's already called there. In xe_display_pm_suspend we have drm_kms_helper_poll_disable and it disables output polling. Moreover we don't have the functions defined for enable/disable_hpd defined in connector_ops which is referenced by drm_kms_helper_poll_enable/disable. What we need is on runtime_suspend the poll work should be enabled so that hotplug can be detected and can come out of runtime_suspend. The same poll work is not required on system suspend. > Also this is missing the xe->info.enable_display check and polling shouldn't be > enabled when system (vs. runtime) suspending. Probably HAS_DISPLAY can be added before calling this intel_hpd_poll_enable. Thanks and Regards, Arun R Murthy -------------------- > > Not sure why the display is suspended/resumed only if d3cold is allowed, for > many of the display s/r steps at least it doesn't make sense to me. > For now moving the d3cold check within > xe_display_pm_suspend()/resume() and enabling/disabling polling regardless of > that flag would be the simplest imo. > > > + > > for_each_gt(gt, xe, id) > > xe_gt_resume(gt); > > > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-07 9:15 ` Murthy, Arun R @ 2024-08-07 14:14 ` Imre Deak 2024-08-08 8:40 ` Jani Nikula 0 siblings, 1 reply; 11+ messages in thread From: Imre Deak @ 2024-08-07 14:14 UTC (permalink / raw) To: Murthy, Arun R Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org On Wed, Aug 07, 2024 at 12:15:33PM +0300, Murthy, Arun R wrote: > > > -----Original Message----- > > From: Deak, Imre <imre.deak@intel.com> > > Sent: Tuesday, August 6, 2024 10:11 PM > > To: Murthy, Arun R <arun.r.murthy@intel.com> > > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > Subject: Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend > > > > On Tue, Aug 06, 2024 at 02:25:13PM +0530, Arun R Murthy wrote: > > > HPD is interrupt based and on runtime suspend change it to polling as > > > HPD is not a wakeup event. A worker thread is scheduled for doing this > > > polling and it keeps polling for HPD live status on an internval of 10s. > > > On runtime resume disable polling and fallback to interrupt mechanism. > > > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> > > > --- > > > drivers/gpu/drm/xe/xe_pm.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c > > > index 9f3c14fd9f33..d952b06ebfb4 100644 > > > --- a/drivers/gpu/drm/xe/xe_pm.c > > > +++ b/drivers/gpu/drm/xe/xe_pm.c > > > @@ -22,6 +22,7 @@ > > > #include "xe_pcode.h" > > > #include "xe_trace.h" > > > #include "xe_wa.h" > > > +#include "intel_hotplug.h" > > > > > > /** > > > * DOC: Xe Power Management > > > @@ -378,6 +379,8 @@ int xe_pm_runtime_suspend(struct xe_device *xe) > > > if (err) > > > goto out; > > > } > > > + /* Enable hpd polling on runtime suspend */ > > > + intel_hpd_poll_enable(xe); > > > > > > xe_irq_suspend(xe); > > > > > > @@ -427,6 +430,9 @@ int xe_pm_runtime_resume(struct xe_device *xe) > > > > > > xe_irq_resume(xe); > > > > > > + /* Disable hpd polling on runtime resume */ > > > + intel_hpd_poll_disable(xe); > > > > This should be handled in xe_display_pm_resume() (likewise enabling polling in > > xe_display_pm_suspend()), in fact it's already called there. > > In xe_display_pm_suspend we have drm_kms_helper_poll_disable and it disables > output polling. What I meant is that intel_hpd_poll_disable() is already called in xe_display_pm_resume(), which you would call now twice during runtime resume if d3cold is allowed. drm_kms_helper_poll_disable() in xe_display_pm_suspend() should be called only during system suspend, which is another thing to fix there. > Moreover we don't have the functions defined for enable/disable_hpd > defined in connector_ops which is referenced by > drm_kms_helper_poll_enable/disable. > > What we need is on runtime_suspend the poll work should be > enabled so that hotplug can be detected and can come out of runtime_suspend. > The same poll work is not required on system suspend. All the display related suspend/resume handling is done already in xe_display_pm_suspend()/resume() or in xe_display.c in general, both for runtime and system suspend/resume. The reason for that is to keep the display specific parts separate from the generic/higher-level device suspend/resume handling in xe_pm.c. New display specific things added should keep this separation. > > Also this is missing the xe->info.enable_display check and polling shouldn't be > > enabled when system (vs. runtime) suspending. > Probably HAS_DISPLAY can be added before calling this intel_hpd_poll_enable. > > Thanks and Regards, > Arun R Murthy > -------------------- > > > > Not sure why the display is suspended/resumed only if d3cold is allowed, for > > many of the display s/r steps at least it doesn't make sense to me. > > For now moving the d3cold check within > > xe_display_pm_suspend()/resume() and enabling/disabling polling regardless of > > that flag would be the simplest imo. > > > > > + > > > for_each_gt(gt, xe, id) > > > xe_gt_resume(gt); > > > > > > -- > > > 2.25.1 > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend 2024-08-07 14:14 ` Imre Deak @ 2024-08-08 8:40 ` Jani Nikula 0 siblings, 0 replies; 11+ messages in thread From: Jani Nikula @ 2024-08-08 8:40 UTC (permalink / raw) To: imre.deak, Murthy, Arun R Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org On Wed, 07 Aug 2024, Imre Deak <imre.deak@intel.com> wrote: > On Wed, Aug 07, 2024 at 12:15:33PM +0300, Murthy, Arun R wrote: >> >> > -----Original Message----- >> > From: Deak, Imre <imre.deak@intel.com> >> > Sent: Tuesday, August 6, 2024 10:11 PM >> > To: Murthy, Arun R <arun.r.murthy@intel.com> >> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org >> > Subject: Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend >> > >> > On Tue, Aug 06, 2024 at 02:25:13PM +0530, Arun R Murthy wrote: >> > > HPD is interrupt based and on runtime suspend change it to polling as >> > > HPD is not a wakeup event. A worker thread is scheduled for doing this >> > > polling and it keeps polling for HPD live status on an internval of 10s. >> > > On runtime resume disable polling and fallback to interrupt mechanism. >> > > >> > > Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> >> > > --- >> > > drivers/gpu/drm/xe/xe_pm.c | 6 ++++++ >> > > 1 file changed, 6 insertions(+) >> > > >> > > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c >> > > index 9f3c14fd9f33..d952b06ebfb4 100644 >> > > --- a/drivers/gpu/drm/xe/xe_pm.c >> > > +++ b/drivers/gpu/drm/xe/xe_pm.c >> > > @@ -22,6 +22,7 @@ >> > > #include "xe_pcode.h" >> > > #include "xe_trace.h" >> > > #include "xe_wa.h" >> > > +#include "intel_hotplug.h" >> > > >> > > /** >> > > * DOC: Xe Power Management >> > > @@ -378,6 +379,8 @@ int xe_pm_runtime_suspend(struct xe_device *xe) >> > > if (err) >> > > goto out; >> > > } >> > > + /* Enable hpd polling on runtime suspend */ >> > > + intel_hpd_poll_enable(xe); >> > > >> > > xe_irq_suspend(xe); >> > > >> > > @@ -427,6 +430,9 @@ int xe_pm_runtime_resume(struct xe_device *xe) >> > > >> > > xe_irq_resume(xe); >> > > >> > > + /* Disable hpd polling on runtime resume */ >> > > + intel_hpd_poll_disable(xe); >> > >> > This should be handled in xe_display_pm_resume() (likewise enabling polling in >> > xe_display_pm_suspend()), in fact it's already called there. >> >> In xe_display_pm_suspend we have drm_kms_helper_poll_disable and it disables >> output polling. > > What I meant is that intel_hpd_poll_disable() is already called in > xe_display_pm_resume(), which you would call now twice during runtime > resume if d3cold is allowed. > > drm_kms_helper_poll_disable() in xe_display_pm_suspend() should be > called only during system suspend, which is another thing to fix there. > >> Moreover we don't have the functions defined for enable/disable_hpd >> defined in connector_ops which is referenced by >> drm_kms_helper_poll_enable/disable. >> >> What we need is on runtime_suspend the poll work should be >> enabled so that hotplug can be detected and can come out of runtime_suspend. >> The same poll work is not required on system suspend. > > All the display related suspend/resume handling is done already in > xe_display_pm_suspend()/resume() or in xe_display.c in general, both for > runtime and system suspend/resume. The reason for that is to keep the > display specific parts separate from the generic/higher-level device > suspend/resume handling in xe_pm.c. New display specific things added > should keep this separation. I'll keep repeating here too: We should aim to move all of this under i915/display, and not duplicate it in i915 and xe specific code. BR, Jani. > >> > Also this is missing the xe->info.enable_display check and polling shouldn't be >> > enabled when system (vs. runtime) suspending. >> Probably HAS_DISPLAY can be added before calling this intel_hpd_poll_enable. >> >> Thanks and Regards, >> Arun R Murthy >> -------------------- >> > >> > Not sure why the display is suspended/resumed only if d3cold is allowed, for >> > many of the display s/r steps at least it doesn't make sense to me. >> > For now moving the d3cold check within >> > xe_display_pm_suspend()/resume() and enabling/disabling polling regardless of >> > that flag would be the simplest imo. >> > >> > > + >> > > for_each_gt(gt, xe, id) >> > > xe_gt_resume(gt); >> > > >> > > -- >> > > 2.25.1 >> > > -- Jani Nikula, Intel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [v2PATCH] drm/xe/display: Change HPD to polling on runtime suspend 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy ` (4 preceding siblings ...) 2024-08-06 16:41 ` Imre Deak @ 2024-08-08 5:56 ` Arun R Murthy 2024-08-08 6:58 ` ✗ CI.Patch_applied: failure for drm/xe/pm: Change HPD to polling on runtime suspend (rev2) Patchwork 6 siblings, 0 replies; 11+ messages in thread From: Arun R Murthy @ 2024-08-08 5:56 UTC (permalink / raw) To: intel-xe, intel-gfx; +Cc: Arun R Murthy HPD is interrupt based and on runtime suspend change it to polling as HPD is not a wakeup event. A worker thread is scheduled for doing this polling and it keeps polling for HPD live status on an internval of 10s. On runtime resume disable polling and fallback to interrupt mechanism. v2: move poll_enable() to xe_display(Imre) Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com> --- drivers/gpu/drm/xe/display/xe_display.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c index 78cccbe28947..989022d21052 100644 --- a/drivers/gpu/drm/xe/display/xe_display.c +++ b/drivers/gpu/drm/xe/display/xe_display.c @@ -315,6 +315,10 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) intel_hpd_cancel_work(xe); + /* Enable hpd polling on runtime suspend */ + if (runtime) + intel_hpd_poll_enable(xe); + intel_suspend_encoders(xe); intel_opregion_suspend(xe, s2idle ? PCI_D1 : PCI_D3cold); -- 2.25.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✗ CI.Patch_applied: failure for drm/xe/pm: Change HPD to polling on runtime suspend (rev2) 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy ` (5 preceding siblings ...) 2024-08-08 5:56 ` [v2PATCH] drm/xe/display: " Arun R Murthy @ 2024-08-08 6:58 ` Patchwork 6 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2024-08-08 6:58 UTC (permalink / raw) To: Arun R Murthy; +Cc: intel-xe == Series Details == Series: drm/xe/pm: Change HPD to polling on runtime suspend (rev2) URL : https://patchwork.freedesktop.org/series/136916/ State : failure == Summary == === Applying kernel patches on branch 'drm-tip' with base: === Base commit: 4f5d551409fb drm-tip: 2024y-08m-07d-12h-38m-22s UTC integration manifest === git am output follows === error: patch failed: drivers/gpu/drm/xe/display/xe_display.c:315 error: drivers/gpu/drm/xe/display/xe_display.c: patch does not apply hint: Use 'git am --show-current-patch=diff' to see the failed patch hint: When you have resolved this problem, run "git am --continue". hint: If you prefer to skip this patch, run "git am --skip" instead. hint: To restore the original branch and stop patching, run "git am --abort". hint: Disable this message with "git config advice.mergeConflict false" Applying: drm/xe/display: Change HPD to polling on runtime suspend Patch failed at 0001 drm/xe/display: Change HPD to polling on runtime suspend ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-08-08 8:41 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-06 8:55 [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend Arun R Murthy 2024-08-06 9:11 ` ✓ CI.Patch_applied: success for " Patchwork 2024-08-06 9:11 ` ✓ CI.checkpatch: " Patchwork 2024-08-06 9:12 ` ✗ CI.KUnit: failure " Patchwork 2024-08-06 16:17 ` [PATCH] " kernel test robot 2024-08-06 16:41 ` Imre Deak 2024-08-07 9:15 ` Murthy, Arun R 2024-08-07 14:14 ` Imre Deak 2024-08-08 8:40 ` Jani Nikula 2024-08-08 5:56 ` [v2PATCH] drm/xe/display: " Arun R Murthy 2024-08-08 6:58 ` ✗ CI.Patch_applied: failure for drm/xe/pm: Change HPD to polling on runtime suspend (rev2) Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox