| Message ID | 20241007140531.1044630-2-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/xe: Separate the d3cold and non-d3cold runtime PM handling | expand |
-----Original Message----- From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak Sent: Monday, October 7, 2024 7:06 AM To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com> Subject: [PATCH 2/2] drm/xe: Add missing HPD interrupt enabling during non-d3cold RPM resume > > Atm the display HPD interrupts that got disabled during runtime > suspend, are re-enabled only if d3cold is enabled. Fix things by > also re-enabling the interrupts if d3cold is disabled. > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> Is there a reason this ended up as a separate patch? Not blocking, just curious. Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > --- > drivers/gpu/drm/xe/display/xe_display.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > index cb2449b7921ac..695c27ac6b0f8 100644 > --- a/drivers/gpu/drm/xe/display/xe_display.c > +++ b/drivers/gpu/drm/xe/display/xe_display.c > @@ -486,6 +486,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > return; > } > > + intel_hpd_init(xe); > intel_hpd_poll_disable(xe); > } > > -- > 2.44.2 > >
On Mon, Oct 07, 2024 at 09:29:46PM +0300, Cavitt, Jonathan wrote: > -----Original Message----- > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > Sent: Monday, October 7, 2024 7:06 AM > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Subject: [PATCH 2/2] drm/xe: Add missing HPD interrupt enabling during non-d3cold RPM resume > > > > Atm the display HPD interrupts that got disabled during runtime > > suspend, are re-enabled only if d3cold is enabled. Fix things by > > also re-enabling the interrupts if d3cold is disabled. > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > Is there a reason this ended up as a separate patch? On the principle of doing one thing in one patch: the first is refactoring, this one is a fix. > Not blocking, just curious. > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > -Jonathan Cavitt > > > --- > > drivers/gpu/drm/xe/display/xe_display.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > > index cb2449b7921ac..695c27ac6b0f8 100644 > > --- a/drivers/gpu/drm/xe/display/xe_display.c > > +++ b/drivers/gpu/drm/xe/display/xe_display.c > > @@ -486,6 +486,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > > return; > > } > > > > + intel_hpd_init(xe); > > intel_hpd_poll_disable(xe); > > } > > > > -- > > 2.44.2 > > > >
On Mon, Oct 07, 2024 at 09:46:53PM +0300, Imre Deak wrote: > On Mon, Oct 07, 2024 at 09:29:46PM +0300, Cavitt, Jonathan wrote: > > -----Original Message----- > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > > Sent: Monday, October 7, 2024 7:06 AM > > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > Subject: [PATCH 2/2] drm/xe: Add missing HPD interrupt enabling during non-d3cold RPM resume > > > > > > Atm the display HPD interrupts that got disabled during runtime > > > suspend, are re-enabled only if d3cold is enabled. Fix things by > > > also re-enabling the interrupts if d3cold is disabled. > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > > Is there a reason this ended up as a separate patch? > > On the principle of doing one thing in one patch: the first is > refactoring, this one is a fix. We do need this, but we need a lot more. Please help on the review of the consolidation and the true split: https://lore.kernel.org/intel-xe/20240924204222.246862-1-rodrigo.vivi@intel.com/T/#m482df5716f81e6f6a3b5f459f57f193a4292ab52 > > > Not blocking, just curious. > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > -Jonathan Cavitt > > > > > --- > > > drivers/gpu/drm/xe/display/xe_display.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > > > index cb2449b7921ac..695c27ac6b0f8 100644 > > > --- a/drivers/gpu/drm/xe/display/xe_display.c > > > +++ b/drivers/gpu/drm/xe/display/xe_display.c > > > @@ -486,6 +486,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > > > return; > > > } > > > > > > + intel_hpd_init(xe); > > > intel_hpd_poll_disable(xe); > > > } > > > > > > -- > > > 2.44.2 > > > > > >
On Mon, Oct 07, 2024 at 02:52:22PM -0400, Rodrigo Vivi wrote: > On Mon, Oct 07, 2024 at 09:46:53PM +0300, Imre Deak wrote: > > On Mon, Oct 07, 2024 at 09:29:46PM +0300, Cavitt, Jonathan wrote: > > > -----Original Message----- > > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > > > Sent: Monday, October 7, 2024 7:06 AM > > > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > > Subject: [PATCH 2/2] drm/xe: Add missing HPD interrupt enabling during non-d3cold RPM resume > > > > > > > > Atm the display HPD interrupts that got disabled during runtime > > > > suspend, are re-enabled only if d3cold is enabled. Fix things by > > > > also re-enabling the interrupts if d3cold is disabled. > > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > > > > Is there a reason this ended up as a separate patch? > > > > On the principle of doing one thing in one patch: the first is > > refactoring, this one is a fix. > > We do need this, but we need a lot more. I think this should be applied before other changes and from your patchset similar fixes (ideally xe-only changes) should be applied first before other refactoring changes (I think there are other such changes there). These fixes could be backported then if needed. > Please help on the review of the consolidation and the true split: > https://lore.kernel.org/intel-xe/20240924204222.246862-1-rodrigo.vivi@intel.com/T/#m482df5716f81e6f6a3b5f459f57f193a4292ab52 > > > > > > Not blocking, just curious. > > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > > -Jonathan Cavitt > > > > > > > --- > > > > drivers/gpu/drm/xe/display/xe_display.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > > > > index cb2449b7921ac..695c27ac6b0f8 100644 > > > > --- a/drivers/gpu/drm/xe/display/xe_display.c > > > > +++ b/drivers/gpu/drm/xe/display/xe_display.c > > > > @@ -486,6 +486,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > > > > return; > > > > } > > > > > > > > + intel_hpd_init(xe); > > > > intel_hpd_poll_disable(xe); > > > > } > > > > > > > > -- > > > > 2.44.2 > > > > > > > >
On Tue, Oct 08, 2024 at 06:33:21PM +0300, Imre Deak wrote: > On Mon, Oct 07, 2024 at 02:52:22PM -0400, Rodrigo Vivi wrote: > > On Mon, Oct 07, 2024 at 09:46:53PM +0300, Imre Deak wrote: > > > On Mon, Oct 07, 2024 at 09:29:46PM +0300, Cavitt, Jonathan wrote: > > > > -----Original Message----- > > > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > > > > Sent: Monday, October 7, 2024 7:06 AM > > > > To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > > > > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > > > Subject: [PATCH 2/2] drm/xe: Add missing HPD interrupt enabling during non-d3cold RPM resume > > > > > > > > > > Atm the display HPD interrupts that got disabled during runtime > > > > > suspend, are re-enabled only if d3cold is enabled. Fix things by > > > > > also re-enabling the interrupts if d3cold is disabled. > > > > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > > > > > > > Is there a reason this ended up as a separate patch? > > > > > > On the principle of doing one thing in one patch: the first is > > > refactoring, this one is a fix. > > > > We do need this, but we need a lot more. > > I think this should be applied before other changes and from your > patchset similar fixes (ideally xe-only changes) should be applied first > before other refactoring changes (I think there are other such changes > there). These fixes could be backported then if needed. Agreed. Let's merge this then and I will address your comments there and get the xe ones before. > > > Please help on the review of the consolidation and the true split: > > https://lore.kernel.org/intel-xe/20240924204222.246862-1-rodrigo.vivi@intel.com/T/#m482df5716f81e6f6a3b5f459f57f193a4292ab52 > > > > > > > > > Not blocking, just curious. > > > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > > > -Jonathan Cavitt > > > > > > > > > --- > > > > > drivers/gpu/drm/xe/display/xe_display.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > > > > > index cb2449b7921ac..695c27ac6b0f8 100644 > > > > > --- a/drivers/gpu/drm/xe/display/xe_display.c > > > > > +++ b/drivers/gpu/drm/xe/display/xe_display.c > > > > > @@ -486,6 +486,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > > > > > return; > > > > > } > > > > > > > > > > + intel_hpd_init(xe); > > > > > intel_hpd_poll_disable(xe); > > > > > } > > > > > > > > > > -- > > > > > 2.44.2 > > > > > > > > > >
diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c index cb2449b7921ac..695c27ac6b0f8 100644 --- a/drivers/gpu/drm/xe/display/xe_display.c +++ b/drivers/gpu/drm/xe/display/xe_display.c @@ -486,6 +486,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) return; } + intel_hpd_init(xe); intel_hpd_poll_disable(xe); }
Atm the display HPD interrupts that got disabled during runtime suspend, are re-enabled only if d3cold is enabled. Fix things by also re-enabling the interrupts if d3cold is disabled. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/xe/display/xe_display.c | 1 + 1 file changed, 1 insertion(+)