[PATCH] drm/xe/pm: Change HPD to polling on runtime suspend

[PATCH] drm/xe/pm: Change HPD to polling on runtime suspend

[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

✓ CI.Patch_applied: success for drm/xe/pm: Change HPD to polling on runtime suspend

[Patchwork]

== 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

✓ CI.checkpatch: success for drm/xe/pm: Change HPD to polling on runtime suspend

[Patchwork]

== 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

✗ CI.KUnit: failure for drm/xe/pm: Change HPD to polling on runtime suspend

[Patchwork]

== 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

Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend

[kernel test robot]

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

Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend

[Imre Deak]

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
> 

RE: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend

[Murthy, Arun R]

> -----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
> >

Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend

[Imre Deak]

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
> > >

[v2PATCH] drm/xe/display: Change HPD to polling on runtime suspend

[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

✗ CI.Patch_applied: failure for drm/xe/pm: Change HPD to polling on runtime suspend (rev2)

[Patchwork]

== 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

Re: [PATCH] drm/xe/pm: Change HPD to polling on runtime suspend

[Jani Nikula]

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