[Intel-xe] [PATCH v2 1/1] fixup! drm/xe/display: Implement display support
Gustavo Sousa
gustavo.sousa at intel.com
Tue Sep 12 13:40:21 UTC 2023
Quoting Lucas De Marchi (2023-09-11 19:16:53-03:00)
>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
Heh, I like _fini just because it is shorter and has 4 characters, making it the
same length as _init :-)
Yep, I agree on having a unified naming style.
>
>>
>>--
>>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>
Thanks for the review! Pushed to drm-xe-next.
Gustavo Sousa
>
>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