[Intel-xe] [PATCH v2 1/1] fixup! drm/xe/display: Implement display support

Lucas De Marchi lucas.demarchi at intel.com
Mon Sep 11 22:16:53 UTC 2023


On Mon, Sep 11, 2023 at 04:50:26PM -0300, Gustavo Sousa wrote:
>Quoting Gustavo Sousa (2023-09-11 16:47:12-03:00)
>>Finalizer counterparts of display initializers are registered with
>>drmm_add_action_or_reset(). Since there are no other explicit users of
>>them outside of xe_display.c, let's turn them into static functions.
>>
>>While at it, since this is a fixup patch, also do other cleanups
>>suggested by Lucas:
>>
>>    * s/xe_display_modset_driver_remove/xe_display_driver_remove/
>>
>>    * Remove the unused xe_display_enable()
>>
>>    * s,xe_display_unlink,xe_display_fini,
>>
>>      The original suggestion for the above was to replace with
>>      xe_display_shutdown, but I went with xe_display_fini to be
>>      consistent with the other "stepped" display finalizers.
>
>Hey Lucas,
>
>Please let me know if your r-b stands with this slight deviation from your
>suggestion.

I personally prefer the _shutdown, but I don't mind. At one point we
have to go back and unify the names we use. See below

>
>--
>Gustavo Sousa
>
>>
>>    * Group functions in the header by namespaces.
>>
>>v2:
>>  - Add more cleanups. (Lucas)
>>
>>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>>Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>>---
>> drivers/gpu/drm/xe/xe_device.c  |  8 ++++----
>> drivers/gpu/drm/xe/xe_display.c | 10 +++++-----
>> drivers/gpu/drm/xe/xe_display.h | 33 ++++++---------------------------
>> 3 files changed, 15 insertions(+), 36 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>>index 109aeb25d19c..a964dc205da9 100644
>>--- a/drivers/gpu/drm/xe/xe_device.c
>>+++ b/drivers/gpu/drm/xe/xe_device.c
>>@@ -342,12 +342,12 @@ int xe_device_probe(struct xe_device *xe)
>>         return 0;
>>
>> err_fini_display:
>>-        xe_display_modset_driver_remove(xe);
>>+        xe_display_driver_remove(xe);
>>
>> err_irq_shutdown:
>>         xe_irq_shutdown(xe);

	   ^^  here

>> err:
>>-        xe_display_unlink(xe);
>>+        xe_display_fini(xe);
>>         return err;
>> }
>>
>>@@ -356,14 +356,14 @@ static void xe_device_remove_display(struct xe_device *xe)
>>         xe_display_unregister(xe);
>>
>>         drm_dev_unplug(&xe->drm);
>>-        xe_display_modset_driver_remove(xe);
>>+        xe_display_driver_remove(xe);
>> }
>>
>> void xe_device_remove(struct xe_device *xe)
>> {
>>         xe_device_remove_display(xe);
>>
>>-        xe_display_unlink(xe);
>>+        xe_display_fini(xe);
>>
>>         xe_irq_shutdown(xe);

	   ^^ and here

but yes, I do see _fini being used in other places.


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

>> }
>>diff --git a/drivers/gpu/drm/xe/xe_display.c b/drivers/gpu/drm/xe/xe_display.c
>>index a453946ad108..37278f73cb39 100644
>>--- a/drivers/gpu/drm/xe/xe_display.c
>>+++ b/drivers/gpu/drm/xe/xe_display.c
>>@@ -132,7 +132,7 @@ int xe_display_create(struct xe_device *xe)
>>         return 0;
>> }
>>
>>-void xe_display_fini_nommio(struct drm_device *dev, void *dummy)
>>+static void xe_display_fini_nommio(struct drm_device *dev, void *dummy)
>> {
>>         struct xe_device *xe = to_xe_device(dev);
>>
>>@@ -164,7 +164,7 @@ int xe_display_init_nommio(struct xe_device *xe)
>>         return drmm_add_action_or_reset(&xe->drm, xe_display_fini_nommio, xe);
>> }
>>
>>-void xe_display_fini_noirq(struct drm_device *dev, void *dummy)
>>+static void xe_display_fini_noirq(struct drm_device *dev, void *dummy)
>> {
>>         struct xe_device *xe = to_xe_device(dev);
>>
>>@@ -204,7 +204,7 @@ int xe_display_init_noirq(struct xe_device *xe)
>>         return drmm_add_action_or_reset(&xe->drm, xe_display_fini_noirq, NULL);
>> }
>>
>>-void xe_display_fini_noaccel(struct drm_device *dev, void *dummy)
>>+static void xe_display_fini_noaccel(struct drm_device *dev, void *dummy)
>> {
>>         struct xe_device *xe = to_xe_device(dev);
>>
>>@@ -236,7 +236,7 @@ int xe_display_init(struct xe_device *xe)
>>         return intel_display_driver_probe(xe);
>> }
>>
>>-void xe_display_unlink(struct xe_device *xe)
>>+void xe_display_fini(struct xe_device *xe)
>> {
>>         if (!xe->info.enable_display)
>>                 return;
>>@@ -269,7 +269,7 @@ void xe_display_unregister(struct xe_device *xe)
>>         intel_display_driver_unregister(xe);
>> }
>>
>>-void xe_display_modset_driver_remove(struct xe_device *xe)
>>+void xe_display_driver_remove(struct xe_device *xe)
>> {
>>         if (!xe->info.enable_display)
>>                 return;
>>diff --git a/drivers/gpu/drm/xe/xe_display.h b/drivers/gpu/drm/xe/xe_display.h
>>index 9e29de012df7..de835d459255 100644
>>--- a/drivers/gpu/drm/xe/xe_display.h
>>+++ b/drivers/gpu/drm/xe/xe_display.h
>>@@ -14,30 +14,23 @@ struct drm_driver;
>>
>> int xe_display_driver_probe_defer(struct pci_dev *pdev);
>> void xe_display_driver_set_hooks(struct drm_driver *driver);
>>+void xe_display_driver_remove(struct xe_device *xe);
>>
>> int xe_display_create(struct xe_device *xe);
>>
>> void xe_display_info_init(struct xe_device *xe);
>>
>> int xe_display_init_nommio(struct xe_device *xe);
>>-void xe_display_fini_nommio(struct drm_device *dev, void *dummy);
>>-
>> int xe_display_init_noirq(struct xe_device *xe);
>>-void xe_display_fini_noirq(struct drm_device *dev, void *dummy);
>>-
>> int xe_display_init_noaccel(struct xe_device *xe);
>>-void xe_display_fini_noaccel(struct drm_device *dev, void *dummy);
>>-
>> int xe_display_init(struct xe_device *xe);
>>-void xe_display_unlink(struct xe_device *xe);
>>+void xe_display_fini(struct xe_device *xe);
>>
>> void xe_display_register(struct xe_device *xe);
>> void xe_display_unregister(struct xe_device *xe);
>>-void xe_display_modset_driver_remove(struct xe_device *xe);
>>
>> void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl);
>> void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir);
>>-
>> void xe_display_irq_reset(struct xe_device *xe);
>> void xe_display_irq_postinstall(struct xe_device *xe, struct xe_gt *gt);
>>
>>@@ -49,35 +42,21 @@ void xe_display_pm_resume(struct xe_device *xe);
>> #else
>>
>> static inline int xe_display_driver_probe_defer(struct pci_dev *pdev) { return 0; }
>>-
>> static inline void xe_display_driver_set_hooks(struct drm_driver *driver) { }
>>+static inline void xe_display_driver_remove(struct xe_device *xe) {}
>>
>>-static inline int
>>-xe_display_create(struct xe_device *xe) { return 0; }
>>+static inline int xe_display_create(struct xe_device *xe) { return 0; }
>>
>> static inline void xe_display_info_init(struct xe_device *xe) { }
>>
>>-static inline int
>>-xe_display_enable(struct pci_dev *pdev, struct drm_driver *driver) { return 0; }
>>-
>>-static inline int
>>-xe_display_init_nommio(struct xe_device *xe) { return 0; }
>>-static inline void xe_display_fini_nommio(struct drm_device *dev, void *dummy) {}
>>-
>>+static inline int xe_display_init_nommio(struct xe_device *xe) { return 0; }
>> static inline int xe_display_init_noirq(struct xe_device *xe) { return 0; }
>>-
>>-static inline void
>>-xe_display_fini_noirq(struct drm_device *dev, void *dummy) {}
>>-
>> static inline int xe_display_init_noaccel(struct xe_device *xe) { return 0; }
>>-static inline void xe_display_fini_noaccel(struct drm_device *dev, void *dummy) {}
>>-
>> static inline int xe_display_init(struct xe_device *xe) { return 0; }
>>-static inline void xe_display_unlink(struct xe_device *xe) {}
>>+static inline void xe_display_fini(struct xe_device *xe) {}
>>
>> static inline void xe_display_register(struct xe_device *xe) {}
>> static inline void xe_display_unregister(struct xe_device *xe) {}
>>-static inline void xe_display_modset_driver_remove(struct xe_device *xe) {}
>>
>> static inline void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl) {}
>> static inline void xe_display_irq_enable(struct xe_device *xe, u32 gu_misc_iir) {}
>>--
>>2.42.0
>>


More information about the Intel-xe mailing list