| Message ID | 20240904173713.26891-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm/xe/display: Merge xe's display info probe and i915 HAS_DISPLAY checks | expand |
-----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo Vivi Sent: Wednesday, September 4, 2024 10:37 AM To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; Nikula, Jani <jani.nikula@intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com> Subject: [RFC] drm/xe/display: Merge xe's display info probe and i915 HAS_DISPLAY checks > > Instead of having multiple checks and the has_display in the middle > of the functions, only execute the Xe display functions if > Xe probed display and pipe_masks still have something valid > after i915's runtime init. > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Apropos of nothing, it would be nice if we could decouple the Xe display driver from the i915 driver, but I don't think this is the best place to do that. LGTM. Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > --- > drivers/gpu/drm/xe/display/xe_display.c | 65 +++++++++++++------------ > 1 file changed, 35 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > index 75736faf2a80..1e248c7aaff0 100644 > --- a/drivers/gpu/drm/xe/display/xe_display.c > +++ b/drivers/gpu/drm/xe/display/xe_display.c > @@ -13,7 +13,6 @@ > #include <uapi/drm/xe_drm.h> > > #include "soc/intel_dram.h" > -#include "i915_drv.h" /* FIXME: HAS_DISPLAY() depends on this */ > #include "intel_acpi.h" > #include "intel_audio.h" > #include "intel_bw.h" > @@ -34,7 +33,12 @@ > > static bool has_display(struct xe_device *xe) > { > - return HAS_DISPLAY(xe); > + /* > + * Xe has probed and configured the display, > + * and some pipes remains enable after > + * __intel_display_device_info_runtime_init() > + */ > + return xe->info.probe_display && HAS_DISPLAY(&xe->display); > } > > /** > @@ -104,7 +108,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) > { > struct xe_device *xe = to_xe_device(dev); > > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_power_domains_cleanup(xe); > @@ -112,7 +116,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) > > int xe_display_init_nommio(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return 0; > > /* Fake uncore lock */ > @@ -129,7 +133,7 @@ static void xe_display_fini_noirq(void *arg) > struct xe_device *xe = arg; > struct intel_display *display = &xe->display; > > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_display_driver_remove_noirq(xe); > @@ -141,7 +145,7 @@ int xe_display_init_noirq(struct xe_device *xe) > struct intel_display *display = &xe->display; > int err; > > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return 0; > > intel_display_driver_early_probe(xe); > @@ -172,7 +176,7 @@ static void xe_display_fini_noaccel(void *arg) > { > struct xe_device *xe = arg; > > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_display_driver_remove_nogem(xe); > @@ -182,7 +186,7 @@ int xe_display_init_noaccel(struct xe_device *xe) > { > int err; > > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return 0; > > err = intel_display_driver_probe_nogem(xe); > @@ -194,7 +198,7 @@ int xe_display_init_noaccel(struct xe_device *xe) > > int xe_display_init(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return 0; > > return intel_display_driver_probe(xe); > @@ -202,7 +206,7 @@ int xe_display_init(struct xe_device *xe) > > void xe_display_fini(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_hpd_poll_fini(xe); > @@ -213,7 +217,7 @@ void xe_display_fini(struct xe_device *xe) > > void xe_display_register(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_display_driver_register(xe); > @@ -223,7 +227,7 @@ void xe_display_register(struct xe_device *xe) > > void xe_display_unregister(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_unregister_dsm_handler(); > @@ -233,7 +237,7 @@ void xe_display_unregister(struct xe_device *xe) > > void xe_display_driver_remove(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_display_driver_remove(xe); > @@ -243,7 +247,7 @@ void xe_display_driver_remove(struct xe_device *xe) > > void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > if (master_ctl & DISPLAY_IRQ) > @@ -254,7 +258,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) > { > struct intel_display *display = &xe->display; > > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > if (gu_misc_iir & GU_MISC_GSE) > @@ -263,7 +267,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) > > void xe_display_irq_reset(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > gen11_display_irq_reset(xe); > @@ -271,7 +275,7 @@ void xe_display_irq_reset(struct xe_device *xe) > > void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > if (gt->info.id == XE_GT0) > @@ -311,7 +315,7 @@ static void xe_display_flush_cleanup_work(struct xe_device *xe) > /* TODO: System and runtime suspend/resume sequences will be sanitized as a follow-up. */ > void xe_display_pm_runtime_suspend(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > if (xe->d3cold.allowed) > @@ -324,7 +328,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > { > struct intel_display *display = &xe->display; > bool s2idle = suspend_to_idle(); > - if (!xe->info.probe_display) > + > + if (!has_display(xe)) > return; > > /* > @@ -333,7 +338,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > */ > intel_power_domains_disable(xe); > intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); > - if (!runtime && has_display(xe)) { > + if (!runtime) { > drm_kms_helper_poll_disable(&xe->drm); > intel_display_driver_disable_user_access(xe); > intel_display_driver_suspend(xe); > @@ -345,7 +350,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > > intel_hpd_cancel_work(xe); > > - if (!runtime && has_display(xe)) { > + if (!runtime) { > intel_display_driver_suspend_access(xe); > intel_encoder_suspend_all(&xe->display); > } > @@ -358,7 +363,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > void xe_display_pm_suspend_late(struct xe_device *xe) > { > bool s2idle = suspend_to_idle(); > - if (!xe->info.probe_display) > + > + if (!has_display(xe)) > return; > > intel_power_domains_suspend(xe, s2idle); > @@ -368,7 +374,7 @@ void xe_display_pm_suspend_late(struct xe_device *xe) > > void xe_display_pm_runtime_resume(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_hpd_poll_disable(xe); > @@ -379,7 +385,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > > void xe_display_pm_resume_early(struct xe_device *xe) > { > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_display_power_resume_early(xe); > @@ -391,23 +397,22 @@ void xe_display_pm_resume(struct xe_device *xe, bool runtime) > { > struct intel_display *display = &xe->display; > > - if (!xe->info.probe_display) > + if (!has_display(xe)) > return; > > intel_dmc_resume(xe); > > - if (has_display(xe)) > - drm_mode_config_reset(&xe->drm); > + drm_mode_config_reset(&xe->drm); > > intel_display_driver_init_hw(xe); > intel_hpd_init(xe); > > - if (!runtime && has_display(xe)) > + if (!runtime) > intel_display_driver_resume_access(xe); > > /* MST sideband requires HPD interrupts enabled */ > intel_dp_mst_resume(xe); > - if (!runtime && has_display(xe)) { > + if (!runtime) { > intel_display_driver_resume(xe); > drm_kms_helper_poll_enable(&xe->drm); > intel_display_driver_enable_user_access(xe); > @@ -441,7 +446,7 @@ int xe_display_probe(struct xe_device *xe) > if (err) > return err; > > - if (has_display(xe)) > + if (HAS_DISPLAY(&xe->display)) > return 0; > > no_display: > -- > 2.46.0 > >
On Wed, Sep 04, 2024 at 01:37:13PM GMT, Rodrigo Vivi wrote: >Instead of having multiple checks and the has_display in the middle >of the functions, only execute the Xe display functions if >Xe probed display and pipe_masks still have something valid >after i915's runtime init. > >Cc: Jani Nikula <jani.nikula@intel.com> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >--- > drivers/gpu/drm/xe/display/xe_display.c | 65 +++++++++++++------------ > 1 file changed, 35 insertions(+), 30 deletions(-) > >diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c >index 75736faf2a80..1e248c7aaff0 100644 >--- a/drivers/gpu/drm/xe/display/xe_display.c >+++ b/drivers/gpu/drm/xe/display/xe_display.c >@@ -13,7 +13,6 @@ > #include <uapi/drm/xe_drm.h> > > #include "soc/intel_dram.h" >-#include "i915_drv.h" /* FIXME: HAS_DISPLAY() depends on this */ > #include "intel_acpi.h" > #include "intel_audio.h" > #include "intel_bw.h" >@@ -34,7 +33,12 @@ > > static bool has_display(struct xe_device *xe) I think has_display is already conflated enough. From the xe side I wouldn't call this has_display if it means something else than "the hardware is present or we know how to drive it". That is the meaning of this flag in drivers/gpu/drm/xe/xe_pci.c and this function here later changed to mean something else :-/ > { >- return HAS_DISPLAY(xe); >+ /* >+ * Xe has probed and configured the display, >+ * and some pipes remains enable after >+ * __intel_display_device_info_runtime_init() >+ */ >+ return xe->info.probe_display && HAS_DISPLAY(&xe->display); I'm not following the motivation here... Shouldn't we remove the HAS_DISPLAY() from here and rather change the display side to do a return-early? if probe_display == 1, from the xe perspective we probed display, we shouldn't be looking at the internal state of display to know what to do on this side of the fence. Lucas De Marchi > } > > /** >@@ -104,7 +108,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) > { > struct xe_device *xe = to_xe_device(dev); > >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_power_domains_cleanup(xe); >@@ -112,7 +116,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) > > int xe_display_init_nommio(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return 0; > > /* Fake uncore lock */ >@@ -129,7 +133,7 @@ static void xe_display_fini_noirq(void *arg) > struct xe_device *xe = arg; > struct intel_display *display = &xe->display; > >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_display_driver_remove_noirq(xe); >@@ -141,7 +145,7 @@ int xe_display_init_noirq(struct xe_device *xe) > struct intel_display *display = &xe->display; > int err; > >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return 0; > > intel_display_driver_early_probe(xe); >@@ -172,7 +176,7 @@ static void xe_display_fini_noaccel(void *arg) > { > struct xe_device *xe = arg; > >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_display_driver_remove_nogem(xe); >@@ -182,7 +186,7 @@ int xe_display_init_noaccel(struct xe_device *xe) > { > int err; > >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return 0; > > err = intel_display_driver_probe_nogem(xe); >@@ -194,7 +198,7 @@ int xe_display_init_noaccel(struct xe_device *xe) > > int xe_display_init(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return 0; > > return intel_display_driver_probe(xe); >@@ -202,7 +206,7 @@ int xe_display_init(struct xe_device *xe) > > void xe_display_fini(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_hpd_poll_fini(xe); >@@ -213,7 +217,7 @@ void xe_display_fini(struct xe_device *xe) > > void xe_display_register(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_display_driver_register(xe); >@@ -223,7 +227,7 @@ void xe_display_register(struct xe_device *xe) > > void xe_display_unregister(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_unregister_dsm_handler(); >@@ -233,7 +237,7 @@ void xe_display_unregister(struct xe_device *xe) > > void xe_display_driver_remove(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_display_driver_remove(xe); >@@ -243,7 +247,7 @@ void xe_display_driver_remove(struct xe_device *xe) > > void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > if (master_ctl & DISPLAY_IRQ) >@@ -254,7 +258,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) > { > struct intel_display *display = &xe->display; > >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > if (gu_misc_iir & GU_MISC_GSE) >@@ -263,7 +267,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) > > void xe_display_irq_reset(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > gen11_display_irq_reset(xe); >@@ -271,7 +275,7 @@ void xe_display_irq_reset(struct xe_device *xe) > > void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > if (gt->info.id == XE_GT0) >@@ -311,7 +315,7 @@ static void xe_display_flush_cleanup_work(struct xe_device *xe) > /* TODO: System and runtime suspend/resume sequences will be sanitized as a follow-up. */ > void xe_display_pm_runtime_suspend(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > if (xe->d3cold.allowed) >@@ -324,7 +328,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > { > struct intel_display *display = &xe->display; > bool s2idle = suspend_to_idle(); >- if (!xe->info.probe_display) >+ >+ if (!has_display(xe)) > return; > > /* >@@ -333,7 +338,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > */ > intel_power_domains_disable(xe); > intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); >- if (!runtime && has_display(xe)) { >+ if (!runtime) { > drm_kms_helper_poll_disable(&xe->drm); > intel_display_driver_disable_user_access(xe); > intel_display_driver_suspend(xe); >@@ -345,7 +350,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > > intel_hpd_cancel_work(xe); > >- if (!runtime && has_display(xe)) { >+ if (!runtime) { > intel_display_driver_suspend_access(xe); > intel_encoder_suspend_all(&xe->display); > } >@@ -358,7 +363,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > void xe_display_pm_suspend_late(struct xe_device *xe) > { > bool s2idle = suspend_to_idle(); >- if (!xe->info.probe_display) >+ >+ if (!has_display(xe)) > return; > > intel_power_domains_suspend(xe, s2idle); >@@ -368,7 +374,7 @@ void xe_display_pm_suspend_late(struct xe_device *xe) > > void xe_display_pm_runtime_resume(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_hpd_poll_disable(xe); >@@ -379,7 +385,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > > void xe_display_pm_resume_early(struct xe_device *xe) > { >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_display_power_resume_early(xe); >@@ -391,23 +397,22 @@ void xe_display_pm_resume(struct xe_device *xe, bool runtime) > { > struct intel_display *display = &xe->display; > >- if (!xe->info.probe_display) >+ if (!has_display(xe)) > return; > > intel_dmc_resume(xe); > >- if (has_display(xe)) >- drm_mode_config_reset(&xe->drm); >+ drm_mode_config_reset(&xe->drm); > > intel_display_driver_init_hw(xe); > intel_hpd_init(xe); > >- if (!runtime && has_display(xe)) >+ if (!runtime) > intel_display_driver_resume_access(xe); > > /* MST sideband requires HPD interrupts enabled */ > intel_dp_mst_resume(xe); >- if (!runtime && has_display(xe)) { >+ if (!runtime) { > intel_display_driver_resume(xe); > drm_kms_helper_poll_enable(&xe->drm); > intel_display_driver_enable_user_access(xe); >@@ -441,7 +446,7 @@ int xe_display_probe(struct xe_device *xe) > if (err) > return err; > >- if (has_display(xe)) >+ if (HAS_DISPLAY(&xe->display)) > return 0; > > no_display: >-- >2.46.0 >
On Thu, Sep 05, 2024 at 08:29:22AM -0500, Lucas De Marchi wrote: > On Wed, Sep 04, 2024 at 01:37:13PM GMT, Rodrigo Vivi wrote: > > Instead of having multiple checks and the has_display in the middle > > of the functions, only execute the Xe display functions if > > Xe probed display and pipe_masks still have something valid > > after i915's runtime init. > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/xe/display/xe_display.c | 65 +++++++++++++------------ > > 1 file changed, 35 insertions(+), 30 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > > index 75736faf2a80..1e248c7aaff0 100644 > > --- a/drivers/gpu/drm/xe/display/xe_display.c > > +++ b/drivers/gpu/drm/xe/display/xe_display.c > > @@ -13,7 +13,6 @@ > > #include <uapi/drm/xe_drm.h> > > > > #include "soc/intel_dram.h" > > -#include "i915_drv.h" /* FIXME: HAS_DISPLAY() depends on this */ > > #include "intel_acpi.h" > > #include "intel_audio.h" > > #include "intel_bw.h" > > @@ -34,7 +33,12 @@ > > > > static bool has_display(struct xe_device *xe) > > I think has_display is already conflated enough. From the xe side I > wouldn't call this has_display if it means something else than "the > hardware is present or we know how to drive it". That is the meaning of > this flag in drivers/gpu/drm/xe/xe_pci.c and this function here later > changed to mean something else :-/ > > > { > > - return HAS_DISPLAY(xe); > > + /* > > + * Xe has probed and configured the display, > > + * and some pipes remains enable after > > + * __intel_display_device_info_runtime_init() > > + */ > > + return xe->info.probe_display && HAS_DISPLAY(&xe->display); > > I'm not following the motivation here... Shouldn't we remove the > HAS_DISPLAY() from here and rather change the display side to do a > return-early? > > if probe_display == 1, from the xe perspective we probed display, we > shouldn't be looking at the internal state of display to know what to do > on this side of the fence. Right, but so far we have other if (has_display()) in the middle of our code. Like Jani reminded me yesterday, we still need to check the state of pipe_mask after runtime init function before many calls. So, the quick goal here was to just consolidate the checks instead of the current sprinkled 'if (has_display()) calls that we have around. So we can proceed with a further clean-up and split on the display calls and then move them out to intel_display and minimize this file as much as we can... https://lore.kernel.org/intel-xe/Zth56C3s8lPQMEBB@intel.com But another question that I'm making myself since yesterday is: Shouldn't we simply kill the Xe's display modprobe option at this point? > > Lucas De Marchi > > > } > > > > /** > > @@ -104,7 +108,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) > > { > > struct xe_device *xe = to_xe_device(dev); > > > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_power_domains_cleanup(xe); > > @@ -112,7 +116,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) > > > > int xe_display_init_nommio(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return 0; > > > > /* Fake uncore lock */ > > @@ -129,7 +133,7 @@ static void xe_display_fini_noirq(void *arg) > > struct xe_device *xe = arg; > > struct intel_display *display = &xe->display; > > > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_display_driver_remove_noirq(xe); > > @@ -141,7 +145,7 @@ int xe_display_init_noirq(struct xe_device *xe) > > struct intel_display *display = &xe->display; > > int err; > > > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return 0; > > > > intel_display_driver_early_probe(xe); > > @@ -172,7 +176,7 @@ static void xe_display_fini_noaccel(void *arg) > > { > > struct xe_device *xe = arg; > > > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_display_driver_remove_nogem(xe); > > @@ -182,7 +186,7 @@ int xe_display_init_noaccel(struct xe_device *xe) > > { > > int err; > > > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return 0; > > > > err = intel_display_driver_probe_nogem(xe); > > @@ -194,7 +198,7 @@ int xe_display_init_noaccel(struct xe_device *xe) > > > > int xe_display_init(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return 0; > > > > return intel_display_driver_probe(xe); > > @@ -202,7 +206,7 @@ int xe_display_init(struct xe_device *xe) > > > > void xe_display_fini(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_hpd_poll_fini(xe); > > @@ -213,7 +217,7 @@ void xe_display_fini(struct xe_device *xe) > > > > void xe_display_register(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_display_driver_register(xe); > > @@ -223,7 +227,7 @@ void xe_display_register(struct xe_device *xe) > > > > void xe_display_unregister(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_unregister_dsm_handler(); > > @@ -233,7 +237,7 @@ void xe_display_unregister(struct xe_device *xe) > > > > void xe_display_driver_remove(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_display_driver_remove(xe); > > @@ -243,7 +247,7 @@ void xe_display_driver_remove(struct xe_device *xe) > > > > void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > if (master_ctl & DISPLAY_IRQ) > > @@ -254,7 +258,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) > > { > > struct intel_display *display = &xe->display; > > > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > if (gu_misc_iir & GU_MISC_GSE) > > @@ -263,7 +267,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) > > > > void xe_display_irq_reset(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > gen11_display_irq_reset(xe); > > @@ -271,7 +275,7 @@ void xe_display_irq_reset(struct xe_device *xe) > > > > void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > if (gt->info.id == XE_GT0) > > @@ -311,7 +315,7 @@ static void xe_display_flush_cleanup_work(struct xe_device *xe) > > /* TODO: System and runtime suspend/resume sequences will be sanitized as a follow-up. */ > > void xe_display_pm_runtime_suspend(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > if (xe->d3cold.allowed) > > @@ -324,7 +328,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > > { > > struct intel_display *display = &xe->display; > > bool s2idle = suspend_to_idle(); > > - if (!xe->info.probe_display) > > + > > + if (!has_display(xe)) > > return; > > > > /* > > @@ -333,7 +338,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > > */ > > intel_power_domains_disable(xe); > > intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); > > - if (!runtime && has_display(xe)) { > > + if (!runtime) { > > drm_kms_helper_poll_disable(&xe->drm); > > intel_display_driver_disable_user_access(xe); > > intel_display_driver_suspend(xe); > > @@ -345,7 +350,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > > > > intel_hpd_cancel_work(xe); > > > > - if (!runtime && has_display(xe)) { > > + if (!runtime) { > > intel_display_driver_suspend_access(xe); > > intel_encoder_suspend_all(&xe->display); > > } > > @@ -358,7 +363,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) > > void xe_display_pm_suspend_late(struct xe_device *xe) > > { > > bool s2idle = suspend_to_idle(); > > - if (!xe->info.probe_display) > > + > > + if (!has_display(xe)) > > return; > > > > intel_power_domains_suspend(xe, s2idle); > > @@ -368,7 +374,7 @@ void xe_display_pm_suspend_late(struct xe_device *xe) > > > > void xe_display_pm_runtime_resume(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_hpd_poll_disable(xe); > > @@ -379,7 +385,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) > > > > void xe_display_pm_resume_early(struct xe_device *xe) > > { > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_display_power_resume_early(xe); > > @@ -391,23 +397,22 @@ void xe_display_pm_resume(struct xe_device *xe, bool runtime) > > { > > struct intel_display *display = &xe->display; > > > > - if (!xe->info.probe_display) > > + if (!has_display(xe)) > > return; > > > > intel_dmc_resume(xe); > > > > - if (has_display(xe)) > > - drm_mode_config_reset(&xe->drm); > > + drm_mode_config_reset(&xe->drm); > > > > intel_display_driver_init_hw(xe); > > intel_hpd_init(xe); > > > > - if (!runtime && has_display(xe)) > > + if (!runtime) > > intel_display_driver_resume_access(xe); > > > > /* MST sideband requires HPD interrupts enabled */ > > intel_dp_mst_resume(xe); > > - if (!runtime && has_display(xe)) { > > + if (!runtime) { > > intel_display_driver_resume(xe); > > drm_kms_helper_poll_enable(&xe->drm); > > intel_display_driver_enable_user_access(xe); > > @@ -441,7 +446,7 @@ int xe_display_probe(struct xe_device *xe) > > if (err) > > return err; > > > > - if (has_display(xe)) > > + if (HAS_DISPLAY(&xe->display)) > > return 0; > > > > no_display: > > -- > > 2.46.0 > >
On Thu, 05 Sep 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Thu, Sep 05, 2024 at 08:29:22AM -0500, Lucas De Marchi wrote: >> On Wed, Sep 04, 2024 at 01:37:13PM GMT, Rodrigo Vivi wrote: >> > Instead of having multiple checks and the has_display in the middle >> > of the functions, only execute the Xe display functions if >> > Xe probed display and pipe_masks still have something valid >> > after i915's runtime init. >> > >> > Cc: Jani Nikula <jani.nikula@intel.com> >> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> > --- >> > drivers/gpu/drm/xe/display/xe_display.c | 65 +++++++++++++------------ >> > 1 file changed, 35 insertions(+), 30 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c >> > index 75736faf2a80..1e248c7aaff0 100644 >> > --- a/drivers/gpu/drm/xe/display/xe_display.c >> > +++ b/drivers/gpu/drm/xe/display/xe_display.c >> > @@ -13,7 +13,6 @@ >> > #include <uapi/drm/xe_drm.h> >> > >> > #include "soc/intel_dram.h" >> > -#include "i915_drv.h" /* FIXME: HAS_DISPLAY() depends on this */ >> > #include "intel_acpi.h" >> > #include "intel_audio.h" >> > #include "intel_bw.h" >> > @@ -34,7 +33,12 @@ >> > >> > static bool has_display(struct xe_device *xe) >> >> I think has_display is already conflated enough. From the xe side I >> wouldn't call this has_display if it means something else than "the >> hardware is present or we know how to drive it". That is the meaning of >> this flag in drivers/gpu/drm/xe/xe_pci.c and this function here later >> changed to mean something else :-/ >> >> > { >> > - return HAS_DISPLAY(xe); >> > + /* >> > + * Xe has probed and configured the display, >> > + * and some pipes remains enable after >> > + * __intel_display_device_info_runtime_init() >> > + */ >> > + return xe->info.probe_display && HAS_DISPLAY(&xe->display); >> >> I'm not following the motivation here... Shouldn't we remove the >> HAS_DISPLAY() from here and rather change the display side to do a >> return-early? >> >> if probe_display == 1, from the xe perspective we probed display, we >> shouldn't be looking at the internal state of display to know what to do >> on this side of the fence. > > Right, but so far we have other if (has_display()) in the middle of our code. > Like Jani reminded me yesterday, we still need to check the state of > pipe_mask after runtime init function before many calls. > > So, the quick goal here was to just consolidate the checks instead of > the current sprinkled 'if (has_display()) calls that we have around. > > So we can proceed with a further clean-up and split on the display > calls and then move them out to intel_display and minimize this > file as much as we can... > > https://lore.kernel.org/intel-xe/Zth56C3s8lPQMEBB@intel.com > > But another question that I'm making myself since yesterday is: > Shouldn't we simply kill the Xe's display modprobe option at this point? That's what my first reaction to the whole parameter was. But apparently it's useful for platform enabling. I think the rename helped in clarifying what it means. BR, Jani. > >> >> Lucas De Marchi >> >> > } >> > >> > /** >> > @@ -104,7 +108,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) >> > { >> > struct xe_device *xe = to_xe_device(dev); >> > >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_power_domains_cleanup(xe); >> > @@ -112,7 +116,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) >> > >> > int xe_display_init_nommio(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return 0; >> > >> > /* Fake uncore lock */ >> > @@ -129,7 +133,7 @@ static void xe_display_fini_noirq(void *arg) >> > struct xe_device *xe = arg; >> > struct intel_display *display = &xe->display; >> > >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_display_driver_remove_noirq(xe); >> > @@ -141,7 +145,7 @@ int xe_display_init_noirq(struct xe_device *xe) >> > struct intel_display *display = &xe->display; >> > int err; >> > >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return 0; >> > >> > intel_display_driver_early_probe(xe); >> > @@ -172,7 +176,7 @@ static void xe_display_fini_noaccel(void *arg) >> > { >> > struct xe_device *xe = arg; >> > >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_display_driver_remove_nogem(xe); >> > @@ -182,7 +186,7 @@ int xe_display_init_noaccel(struct xe_device *xe) >> > { >> > int err; >> > >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return 0; >> > >> > err = intel_display_driver_probe_nogem(xe); >> > @@ -194,7 +198,7 @@ int xe_display_init_noaccel(struct xe_device *xe) >> > >> > int xe_display_init(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return 0; >> > >> > return intel_display_driver_probe(xe); >> > @@ -202,7 +206,7 @@ int xe_display_init(struct xe_device *xe) >> > >> > void xe_display_fini(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_hpd_poll_fini(xe); >> > @@ -213,7 +217,7 @@ void xe_display_fini(struct xe_device *xe) >> > >> > void xe_display_register(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_display_driver_register(xe); >> > @@ -223,7 +227,7 @@ void xe_display_register(struct xe_device *xe) >> > >> > void xe_display_unregister(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_unregister_dsm_handler(); >> > @@ -233,7 +237,7 @@ void xe_display_unregister(struct xe_device *xe) >> > >> > void xe_display_driver_remove(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_display_driver_remove(xe); >> > @@ -243,7 +247,7 @@ void xe_display_driver_remove(struct xe_device *xe) >> > >> > void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > if (master_ctl & DISPLAY_IRQ) >> > @@ -254,7 +258,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) >> > { >> > struct intel_display *display = &xe->display; >> > >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > if (gu_misc_iir & GU_MISC_GSE) >> > @@ -263,7 +267,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) >> > >> > void xe_display_irq_reset(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > gen11_display_irq_reset(xe); >> > @@ -271,7 +275,7 @@ void xe_display_irq_reset(struct xe_device *xe) >> > >> > void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > if (gt->info.id == XE_GT0) >> > @@ -311,7 +315,7 @@ static void xe_display_flush_cleanup_work(struct xe_device *xe) >> > /* TODO: System and runtime suspend/resume sequences will be sanitized as a follow-up. */ >> > void xe_display_pm_runtime_suspend(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > if (xe->d3cold.allowed) >> > @@ -324,7 +328,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) >> > { >> > struct intel_display *display = &xe->display; >> > bool s2idle = suspend_to_idle(); >> > - if (!xe->info.probe_display) >> > + >> > + if (!has_display(xe)) >> > return; >> > >> > /* >> > @@ -333,7 +338,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) >> > */ >> > intel_power_domains_disable(xe); >> > intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); >> > - if (!runtime && has_display(xe)) { >> > + if (!runtime) { >> > drm_kms_helper_poll_disable(&xe->drm); >> > intel_display_driver_disable_user_access(xe); >> > intel_display_driver_suspend(xe); >> > @@ -345,7 +350,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) >> > >> > intel_hpd_cancel_work(xe); >> > >> > - if (!runtime && has_display(xe)) { >> > + if (!runtime) { >> > intel_display_driver_suspend_access(xe); >> > intel_encoder_suspend_all(&xe->display); >> > } >> > @@ -358,7 +363,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) >> > void xe_display_pm_suspend_late(struct xe_device *xe) >> > { >> > bool s2idle = suspend_to_idle(); >> > - if (!xe->info.probe_display) >> > + >> > + if (!has_display(xe)) >> > return; >> > >> > intel_power_domains_suspend(xe, s2idle); >> > @@ -368,7 +374,7 @@ void xe_display_pm_suspend_late(struct xe_device *xe) >> > >> > void xe_display_pm_runtime_resume(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_hpd_poll_disable(xe); >> > @@ -379,7 +385,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) >> > >> > void xe_display_pm_resume_early(struct xe_device *xe) >> > { >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_display_power_resume_early(xe); >> > @@ -391,23 +397,22 @@ void xe_display_pm_resume(struct xe_device *xe, bool runtime) >> > { >> > struct intel_display *display = &xe->display; >> > >> > - if (!xe->info.probe_display) >> > + if (!has_display(xe)) >> > return; >> > >> > intel_dmc_resume(xe); >> > >> > - if (has_display(xe)) >> > - drm_mode_config_reset(&xe->drm); >> > + drm_mode_config_reset(&xe->drm); >> > >> > intel_display_driver_init_hw(xe); >> > intel_hpd_init(xe); >> > >> > - if (!runtime && has_display(xe)) >> > + if (!runtime) >> > intel_display_driver_resume_access(xe); >> > >> > /* MST sideband requires HPD interrupts enabled */ >> > intel_dp_mst_resume(xe); >> > - if (!runtime && has_display(xe)) { >> > + if (!runtime) { >> > intel_display_driver_resume(xe); >> > drm_kms_helper_poll_enable(&xe->drm); >> > intel_display_driver_enable_user_access(xe); >> > @@ -441,7 +446,7 @@ int xe_display_probe(struct xe_device *xe) >> > if (err) >> > return err; >> > >> > - if (has_display(xe)) >> > + if (HAS_DISPLAY(&xe->display)) >> > return 0; >> > >> > no_display: >> > -- >> > 2.46.0 >> >
diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c index 75736faf2a80..1e248c7aaff0 100644 --- a/drivers/gpu/drm/xe/display/xe_display.c +++ b/drivers/gpu/drm/xe/display/xe_display.c @@ -13,7 +13,6 @@ #include <uapi/drm/xe_drm.h> #include "soc/intel_dram.h" -#include "i915_drv.h" /* FIXME: HAS_DISPLAY() depends on this */ #include "intel_acpi.h" #include "intel_audio.h" #include "intel_bw.h" @@ -34,7 +33,12 @@ static bool has_display(struct xe_device *xe) { - return HAS_DISPLAY(xe); + /* + * Xe has probed and configured the display, + * and some pipes remains enable after + * __intel_display_device_info_runtime_init() + */ + return xe->info.probe_display && HAS_DISPLAY(&xe->display); } /** @@ -104,7 +108,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) { struct xe_device *xe = to_xe_device(dev); - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_power_domains_cleanup(xe); @@ -112,7 +116,7 @@ static void xe_display_fini_nommio(struct drm_device *dev, void *dummy) int xe_display_init_nommio(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return 0; /* Fake uncore lock */ @@ -129,7 +133,7 @@ static void xe_display_fini_noirq(void *arg) struct xe_device *xe = arg; struct intel_display *display = &xe->display; - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_display_driver_remove_noirq(xe); @@ -141,7 +145,7 @@ int xe_display_init_noirq(struct xe_device *xe) struct intel_display *display = &xe->display; int err; - if (!xe->info.probe_display) + if (!has_display(xe)) return 0; intel_display_driver_early_probe(xe); @@ -172,7 +176,7 @@ static void xe_display_fini_noaccel(void *arg) { struct xe_device *xe = arg; - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_display_driver_remove_nogem(xe); @@ -182,7 +186,7 @@ int xe_display_init_noaccel(struct xe_device *xe) { int err; - if (!xe->info.probe_display) + if (!has_display(xe)) return 0; err = intel_display_driver_probe_nogem(xe); @@ -194,7 +198,7 @@ int xe_display_init_noaccel(struct xe_device *xe) int xe_display_init(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return 0; return intel_display_driver_probe(xe); @@ -202,7 +206,7 @@ int xe_display_init(struct xe_device *xe) void xe_display_fini(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_hpd_poll_fini(xe); @@ -213,7 +217,7 @@ void xe_display_fini(struct xe_device *xe) void xe_display_register(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_display_driver_register(xe); @@ -223,7 +227,7 @@ void xe_display_register(struct xe_device *xe) void xe_display_unregister(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_unregister_dsm_handler(); @@ -233,7 +237,7 @@ void xe_display_unregister(struct xe_device *xe) void xe_display_driver_remove(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_display_driver_remove(xe); @@ -243,7 +247,7 @@ void xe_display_driver_remove(struct xe_device *xe) void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; if (master_ctl & DISPLAY_IRQ) @@ -254,7 +258,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) { struct intel_display *display = &xe->display; - if (!xe->info.probe_display) + if (!has_display(xe)) return; if (gu_misc_iir & GU_MISC_GSE) @@ -263,7 +267,7 @@ void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) void xe_display_irq_reset(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; gen11_display_irq_reset(xe); @@ -271,7 +275,7 @@ void xe_display_irq_reset(struct xe_device *xe) void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; if (gt->info.id == XE_GT0) @@ -311,7 +315,7 @@ static void xe_display_flush_cleanup_work(struct xe_device *xe) /* TODO: System and runtime suspend/resume sequences will be sanitized as a follow-up. */ void xe_display_pm_runtime_suspend(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; if (xe->d3cold.allowed) @@ -324,7 +328,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) { struct intel_display *display = &xe->display; bool s2idle = suspend_to_idle(); - if (!xe->info.probe_display) + + if (!has_display(xe)) return; /* @@ -333,7 +338,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) */ intel_power_domains_disable(xe); intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); - if (!runtime && has_display(xe)) { + if (!runtime) { drm_kms_helper_poll_disable(&xe->drm); intel_display_driver_disable_user_access(xe); intel_display_driver_suspend(xe); @@ -345,7 +350,7 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) intel_hpd_cancel_work(xe); - if (!runtime && has_display(xe)) { + if (!runtime) { intel_display_driver_suspend_access(xe); intel_encoder_suspend_all(&xe->display); } @@ -358,7 +363,8 @@ void xe_display_pm_suspend(struct xe_device *xe, bool runtime) void xe_display_pm_suspend_late(struct xe_device *xe) { bool s2idle = suspend_to_idle(); - if (!xe->info.probe_display) + + if (!has_display(xe)) return; intel_power_domains_suspend(xe, s2idle); @@ -368,7 +374,7 @@ void xe_display_pm_suspend_late(struct xe_device *xe) void xe_display_pm_runtime_resume(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_hpd_poll_disable(xe); @@ -379,7 +385,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe) void xe_display_pm_resume_early(struct xe_device *xe) { - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_display_power_resume_early(xe); @@ -391,23 +397,22 @@ void xe_display_pm_resume(struct xe_device *xe, bool runtime) { struct intel_display *display = &xe->display; - if (!xe->info.probe_display) + if (!has_display(xe)) return; intel_dmc_resume(xe); - if (has_display(xe)) - drm_mode_config_reset(&xe->drm); + drm_mode_config_reset(&xe->drm); intel_display_driver_init_hw(xe); intel_hpd_init(xe); - if (!runtime && has_display(xe)) + if (!runtime) intel_display_driver_resume_access(xe); /* MST sideband requires HPD interrupts enabled */ intel_dp_mst_resume(xe); - if (!runtime && has_display(xe)) { + if (!runtime) { intel_display_driver_resume(xe); drm_kms_helper_poll_enable(&xe->drm); intel_display_driver_enable_user_access(xe); @@ -441,7 +446,7 @@ int xe_display_probe(struct xe_device *xe) if (err) return err; - if (has_display(xe)) + if (HAS_DISPLAY(&xe->display)) return 0; no_display:
Instead of having multiple checks and the has_display in the middle of the functions, only execute the Xe display functions if Xe probed display and pipe_masks still have something valid after i915's runtime init. Cc: Jani Nikula <jani.nikula@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/xe/display/xe_display.c | 65 +++++++++++++------------ 1 file changed, 35 insertions(+), 30 deletions(-)