* [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver
@ 2024-09-09 4:03 Bommu Krishnaiah
2024-09-09 4:50 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Bommu Krishnaiah @ 2024-09-09 4:03 UTC (permalink / raw)
To: intel-xe
Cc: intel-gfx, Bommu Krishnaiah, Kamil Konieczny,
Daniele Ceraolo Spurio, Lucas De Marchi, Tejas Upadhyay
This update addresses the unload/reload sequence of MEI modules in relation to
the i915/Xe graphics driver. On platforms where the MEI hardware is integrated
with the graphics device (e.g., DG2/BMG), the i915/xe driver is depend on the MEI
modules. Conversely, on newer platforms like MTL and LNL, where the MEI hardware
is separate, this dependency does not exist.
The changes introduced ensure that MEI modules are unloaded and reloaded in the
correct order based on platform-specific dependencies. This is achieved by adding
a MODULE_SOFTDEP directive to the i915 and Xe module code.
These changes enhance the robustness of MEI module handling across different hardware
platforms, ensuring that the i915/Xe driver can be cleanly unloaded and reloaded
without issues.
v2: updated commit message
Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
---
drivers/gpu/drm/i915/i915_module.c | 2 ++
drivers/gpu/drm/xe/xe_module.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
index 65acd7bf75d0..2ad079ad35db 100644
--- a/drivers/gpu/drm/i915/i915_module.c
+++ b/drivers/gpu/drm/i915/i915_module.c
@@ -75,6 +75,8 @@ static const struct {
};
static int init_progress;
+MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc");
+
static int __init i915_init(void)
{
int err, i;
diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
index bfc3deebdaa2..5633ea1841b7 100644
--- a/drivers/gpu/drm/xe/xe_module.c
+++ b/drivers/gpu/drm/xe/xe_module.c
@@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i)
init_funcs[i].exit();
}
+MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc");
+
static int __init xe_init(void)
{
int err, i;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-09 4:03 [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver Bommu Krishnaiah @ 2024-09-09 4:50 ` Patchwork 2024-09-09 5:11 ` ✗ Fi.CI.BAT: failure " Patchwork 2024-09-10 15:03 ` [PATCH v2] " Rodrigo Vivi 2 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2024-09-09 4:50 UTC (permalink / raw) To: Bommu Krishnaiah; +Cc: intel-gfx == Series Details == Series: drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver URL : https://patchwork.freedesktop.org/series/138379/ State : warning == Summary == Error: dim checkpatch failed 01263a09e993 drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver -:7: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #7: This update addresses the unload/reload sequence of MEI modules in relation to total: 0 errors, 1 warnings, 0 checks, 16 lines checked ^ permalink raw reply [flat|nested] 11+ messages in thread
* ✗ Fi.CI.BAT: failure for drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-09 4:03 [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver Bommu Krishnaiah 2024-09-09 4:50 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork @ 2024-09-09 5:11 ` Patchwork 2024-09-10 15:03 ` [PATCH v2] " Rodrigo Vivi 2 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2024-09-09 5:11 UTC (permalink / raw) To: Bommu Krishnaiah; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 3835 bytes --] == Series Details == Series: drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver URL : https://patchwork.freedesktop.org/series/138379/ State : failure == Summary == CI Bug Log - changes from CI_DRM_15378 -> Patchwork_138379v1 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_138379v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_138379v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/index.html Participating hosts (43 -> 39) ------------------------------ Missing (4): bat-jsl-1 bat-arlh-3 bat-atsm-1 fi-snb-2520m Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_138379v1: ### IGT changes ### #### Possible regressions #### * igt@i915_selftest@live: - bat-adlm-1: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15378/bat-adlm-1/igt@i915_selftest@live.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/bat-adlm-1/igt@i915_selftest@live.html - bat-twl-2: [PASS][3] -> [INCOMPLETE][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15378/bat-twl-2/igt@i915_selftest@live.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/bat-twl-2/igt@i915_selftest@live.html Known issues ------------ Here are the changes found in Patchwork_138379v1 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@fbdev@read: - bat-arls-1: [PASS][5] -> [DMESG-WARN][6] ([i915#12102]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15378/bat-arls-1/igt@fbdev@read.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/bat-arls-1/igt@fbdev@read.html * igt@i915_selftest@live@gt_lrc: - bat-twl-2: [PASS][7] -> [INCOMPLETE][8] ([i915#11981]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15378/bat-twl-2/igt@i915_selftest@live@gt_lrc.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/bat-twl-2/igt@i915_selftest@live@gt_lrc.html * igt@i915_selftest@live@hangcheck: - bat-adlm-1: [PASS][9] -> [INCOMPLETE][10] ([i915#9413]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15378/bat-adlm-1/igt@i915_selftest@live@hangcheck.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/bat-adlm-1/igt@i915_selftest@live@hangcheck.html #### Possible fixes #### * igt@fbdev@info: - bat-arls-1: [DMESG-WARN][11] ([i915#12102]) -> [PASS][12] [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15378/bat-arls-1/igt@fbdev@info.html [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/bat-arls-1/igt@fbdev@info.html [i915#11981]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11981 [i915#12102]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12102 [i915#9413]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9413 Build changes ------------- * Linux: CI_DRM_15378 -> Patchwork_138379v1 CI-20190529: 20190529 CI_DRM_15378: 3d2b2701d3c250fff370f5057f5a9d88d8ba6ca6 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_8011: 26dca87f1252b7f6f0c0f833050256f0244d61e9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_138379v1: 3d2b2701d3c250fff370f5057f5a9d88d8ba6ca6 @ git://anongit.freedesktop.org/gfx-ci/linux == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_138379v1/index.html [-- Attachment #2: Type: text/html, Size: 4632 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-09 4:03 [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver Bommu Krishnaiah 2024-09-09 4:50 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork 2024-09-09 5:11 ` ✗ Fi.CI.BAT: failure " Patchwork @ 2024-09-10 15:03 ` Rodrigo Vivi 2024-09-10 15:43 ` Lucas De Marchi 2 siblings, 1 reply; 11+ messages in thread From: Rodrigo Vivi @ 2024-09-10 15:03 UTC (permalink / raw) To: Bommu Krishnaiah Cc: intel-xe, intel-gfx, Kamil Konieczny, Daniele Ceraolo Spurio, Lucas De Marchi, Tejas Upadhyay, Tvrtko Ursulin, Joonas Lahtinen, Jani Nikula, Thomas Hellström, Alan Previn, Tomas Winkler, Alexander Usyskin On Mon, Sep 09, 2024 at 09:33:17AM +0530, Bommu Krishnaiah wrote: > This update addresses the unload/reload sequence of MEI modules in relation to > the i915/Xe graphics driver. On platforms where the MEI hardware is integrated > with the graphics device (e.g., DG2/BMG), the i915/xe driver is depend on the MEI > modules. Conversely, on newer platforms like MTL and LNL, where the MEI hardware > is separate, this dependency does not exist. > > The changes introduced ensure that MEI modules are unloaded and reloaded in the > correct order based on platform-specific dependencies. This is achieved by adding > a MODULE_SOFTDEP directive to the i915 and Xe module code. > > These changes enhance the robustness of MEI module handling across different hardware > platforms, ensuring that the i915/Xe driver can be cleanly unloaded and reloaded > without issues. > > v2: updated commit message > > Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com> > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > --- > drivers/gpu/drm/i915/i915_module.c | 2 ++ > drivers/gpu/drm/xe/xe_module.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c > index 65acd7bf75d0..2ad079ad35db 100644 > --- a/drivers/gpu/drm/i915/i915_module.c > +++ b/drivers/gpu/drm/i915/i915_module.c > @@ -75,6 +75,8 @@ static const struct { > }; > static int init_progress; > > +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > + > static int __init i915_init(void) > { > int err, i; > diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c > index bfc3deebdaa2..5633ea1841b7 100644 > --- a/drivers/gpu/drm/xe/xe_module.c > +++ b/drivers/gpu/drm/xe/xe_module.c > @@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i) > init_funcs[i].exit(); > } > > +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); I'm honestly not very comfortable with this. 1. This is not true for every device supported by these modules. 2. This is not true for every (and the most basic) functionality of these drivers. Shouldn't this be done in the the mei side? Couldn't at probe we identify the need of them and if needed we return -EPROBE to attempt a retry after the mei drivers were probed? Cc: Alexander Usyskin <alexander.usyskin@intel.com> Cc: Tomas Winkler <tomas.winkler@intel.com> Cc: Alan Previn <alan.previn.teres.alexis@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Tvrtko Ursulin <tursulin@ursulin.net> > + > static int __init xe_init(void) > { > int err, i; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-10 15:03 ` [PATCH v2] " Rodrigo Vivi @ 2024-09-10 15:43 ` Lucas De Marchi 2024-09-11 6:00 ` Bommu, Krishnaiah 0 siblings, 1 reply; 11+ messages in thread From: Lucas De Marchi @ 2024-09-10 15:43 UTC (permalink / raw) To: Rodrigo Vivi Cc: Bommu Krishnaiah, intel-xe, intel-gfx, Kamil Konieczny, Daniele Ceraolo Spurio, Tejas Upadhyay, Tvrtko Ursulin, Joonas Lahtinen, Jani Nikula, Thomas Hellström, Alan Previn, Tomas Winkler, Alexander Usyskin On Tue, Sep 10, 2024 at 11:03:30AM GMT, Rodrigo Vivi wrote: >On Mon, Sep 09, 2024 at 09:33:17AM +0530, Bommu Krishnaiah wrote: >> This update addresses the unload/reload sequence of MEI modules in relation to >> the i915/Xe graphics driver. On platforms where the MEI hardware is integrated >> with the graphics device (e.g., DG2/BMG), the i915/xe driver is depend on the MEI >> modules. Conversely, on newer platforms like MTL and LNL, where the MEI hardware >> is separate, this dependency does not exist. >> >> The changes introduced ensure that MEI modules are unloaded and reloaded in the >> correct order based on platform-specific dependencies. This is achieved by adding >> a MODULE_SOFTDEP directive to the i915 and Xe module code. can you explain what causes the modules to be loaded today? Also, is this to fix anything related to *loading* order or just unload? >> >> These changes enhance the robustness of MEI module handling across different hardware >> platforms, ensuring that the i915/Xe driver can be cleanly unloaded and reloaded >> without issues. >> >> v2: updated commit message >> >> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com> >> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> >> --- >> drivers/gpu/drm/i915/i915_module.c | 2 ++ >> drivers/gpu/drm/xe/xe_module.c | 2 ++ >> 2 files changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c >> index 65acd7bf75d0..2ad079ad35db 100644 >> --- a/drivers/gpu/drm/i915/i915_module.c >> +++ b/drivers/gpu/drm/i915/i915_module.c >> @@ -75,6 +75,8 @@ static const struct { >> }; >> static int init_progress; >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); >> + >> static int __init i915_init(void) >> { >> int err, i; >> diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c >> index bfc3deebdaa2..5633ea1841b7 100644 >> --- a/drivers/gpu/drm/xe/xe_module.c >> +++ b/drivers/gpu/drm/xe/xe_module.c >> @@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i) >> init_funcs[i].exit(); >> } >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > >I'm honestly not very comfortable with this. > >1. This is not true for every device supported by these modules. >2. This is not true for every (and the most basic) functionality of these drivers. > >Shouldn't this be done in the the mei side? I don't think it's possible to do from the mei side. Would mei depend on both xe and i915 (and thus cause both to be loaded regardless of the platform?). For a runtime dependency like this that depends on the platform, I think the best way would be a weakdep + either a request_module() or something else that causes the module to load (is that what comp_* is doing today?) > >Couldn't at probe we identify the need of them and if needed we return -EPROBE to >attempt a retry after the mei drivers were probed? I'm not sure this is fixing anything for probe. I think we already wait on the other component to be ready without blocking the rest of the driver functionality. A weakdep wouldn't cause the module to be loaded where it's not needed, but need some clarification if this is trying to fix anything load-related or just unload. Lucas De Marchi > >Cc: Alexander Usyskin <alexander.usyskin@intel.com> >Cc: Tomas Winkler <tomas.winkler@intel.com> >Cc: Alan Previn <alan.previn.teres.alexis@intel.com> >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >Cc: Jani Nikula <jani.nikula@intel.com> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >Cc: Tvrtko Ursulin <tursulin@ursulin.net> > >> + >> static int __init xe_init(void) >> { >> int err, i; >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-10 15:43 ` Lucas De Marchi @ 2024-09-11 6:00 ` Bommu, Krishnaiah 2024-09-11 16:18 ` Lucas De Marchi 2024-09-11 20:41 ` Rodrigo Vivi 0 siblings, 2 replies; 11+ messages in thread From: Bommu, Krishnaiah @ 2024-09-11 6:00 UTC (permalink / raw) To: De Marchi, Lucas, Vivi, Rodrigo Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Kamil Konieczny, Ceraolo Spurio, Daniele, Upadhyay, Tejas, Tvrtko Ursulin, Joonas Lahtinen, Nikula, Jani, Thomas Hellström, Teres Alexis, Alan Previn, Winkler, Tomas, Usyskin, Alexander > -----Original Message----- > From: De Marchi, Lucas <lucas.demarchi@intel.com> > Sent: Tuesday, September 10, 2024 9:13 PM > To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > Cc: Bommu, Krishnaiah <krishnaiah.bommu@intel.com>; intel- > xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kamil Konieczny > <kamil.konieczny@linux.intel.com>; Ceraolo Spurio, Daniele > <daniele.ceraolospurio@intel.com>; Upadhyay, Tejas > <tejas.upadhyay@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; Joonas > Lahtinen <joonas.lahtinen@linux.intel.com>; Nikula, Jani > <jani.nikula@intel.com>; Thomas Hellström > <thomas.hellstrom@linux.intel.com>; Teres Alexis, Alan Previn > <alan.previn.teres.alexis@intel.com>; Winkler, Tomas > <tomas.winkler@intel.com>; Usyskin, Alexander > <alexander.usyskin@intel.com> > Subject: Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI > Modules for i915/Xe Driver > > On Tue, Sep 10, 2024 at 11:03:30AM GMT, Rodrigo Vivi wrote: > >On Mon, Sep 09, 2024 at 09:33:17AM +0530, Bommu Krishnaiah wrote: > >> This update addresses the unload/reload sequence of MEI modules in > >> relation to the i915/Xe graphics driver. On platforms where the MEI > >> hardware is integrated with the graphics device (e.g., DG2/BMG), the > >> i915/xe driver is depend on the MEI modules. Conversely, on newer > >> platforms like MTL and LNL, where the MEI hardware is separate, this > dependency does not exist. > >> > >> The changes introduced ensure that MEI modules are unloaded and > >> reloaded in the correct order based on platform-specific > >> dependencies. This is achieved by adding a MODULE_SOFTDEP directive to > the i915 and Xe module code. > > > can you explain what causes the modules to be loaded today? Also, is this to fix > anything related to *loading* order or just unload? > > >> > >> These changes enhance the robustness of MEI module handling across > >> different hardware platforms, ensuring that the i915/Xe driver can be > >> cleanly unloaded and reloaded without issues. > >> > >> v2: updated commit message > >> > >> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com> > >> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> > >> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_module.c | 2 ++ > >> drivers/gpu/drm/xe/xe_module.c | 2 ++ > >> 2 files changed, 4 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_module.c > >> b/drivers/gpu/drm/i915/i915_module.c > >> index 65acd7bf75d0..2ad079ad35db 100644 > >> --- a/drivers/gpu/drm/i915/i915_module.c > >> +++ b/drivers/gpu/drm/i915/i915_module.c > >> @@ -75,6 +75,8 @@ static const struct { }; static int > >> init_progress; > >> > >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > >> + > >> static int __init i915_init(void) > >> { > >> int err, i; > >> diff --git a/drivers/gpu/drm/xe/xe_module.c > >> b/drivers/gpu/drm/xe/xe_module.c index bfc3deebdaa2..5633ea1841b7 > >> 100644 > >> --- a/drivers/gpu/drm/xe/xe_module.c > >> +++ b/drivers/gpu/drm/xe/xe_module.c > >> @@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i) > >> init_funcs[i].exit(); > >> } > >> > >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > > > >I'm honestly not very comfortable with this. > > > >1. This is not true for every device supported by these modules. > >2. This is not true for every (and the most basic) functionality of these drivers. > > > >Shouldn't this be done in the the mei side? > > I don't think it's possible to do from the mei side. Would mei depend on both xe > and i915 (and thus cause both to be loaded regardless of the platform?). For a > runtime dependency like this that depends on the platform, I think the best way > would be a weakdep + either a request_module() or something else that causes > the module to load (is that what comp_* is doing today?) > > > > >Couldn't at probe we identify the need of them and if needed we return > >-EPROBE to attempt a retry after the mei drivers were probed? > > I'm not sure this is fixing anything for probe. I think we already wait on the other > component to be ready without blocking the rest of the driver functionality. > > A weakdep wouldn't cause the module to be loaded where it's not needed, but > need some clarification if this is trying to fix anything load-related or just unload. This change is fixing unload. During xe load I am seeing mei_gsc modules was loaded, but not unloaded during the unload xe root@DUT6127BMGFRD:/home/gta# lsmod | grep xe ------>>>just after system reboot root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# lsmod | grep mei mei_hdcp 28672 0 mei_pxp 16384 0 mei_me 49152 2 mei 167936 5 mei_hdcp,mei_pxp,mei_me root@DUT6127BMGFRD:/home/gta# lsmod | grep xe root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# modprobe xe root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# lsmod | grep mei mei_gsc_proxy 16384 0 mei_gsc 12288 1 mei_hdcp 28672 0 mei_pxp 16384 0 mei_me 49152 3 mei_gsc mei 167936 8 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# init 3 root@DUT6127BMGFRD:/home/gta# echo -n auto > /sys/bus/pci/devices/0000\:03\:00.0/power/control root@DUT6127BMGFRD:/home/gta# echo -n "0000:03:00.0" > /sys/bus/pci/drivers/xe/unbind root@DUT6127BMGFRD:/home/gta# modprobe -r xe root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# lsmod | grep xe root@DUT6127BMGFRD:/home/gta# lsmod | grep mei mei_gsc_proxy 16384 0 mei_gsc 12288 0 mei_hdcp 28672 0 mei_pxp 16384 0 mei_me 49152 3 mei_gsc mei 167936 7 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me root@DUT6127BMGFRD:/home/gta# Regards, Krishna. > > Lucas De Marchi > > > > >Cc: Alexander Usyskin <alexander.usyskin@intel.com> > >Cc: Tomas Winkler <tomas.winkler@intel.com> > >Cc: Alan Previn <alan.previn.teres.alexis@intel.com> > >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >Cc: Lucas De Marchi <lucas.demarchi@intel.com> > >Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > >Cc: Jani Nikula <jani.nikula@intel.com> > >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >Cc: Tvrtko Ursulin <tursulin@ursulin.net> > > > >> + > >> static int __init xe_init(void) > >> { > >> int err, i; > >> -- > >> 2.25.1 > >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-11 6:00 ` Bommu, Krishnaiah @ 2024-09-11 16:18 ` Lucas De Marchi 2024-09-12 11:58 ` Bommu, Krishnaiah 2024-09-11 20:41 ` Rodrigo Vivi 1 sibling, 1 reply; 11+ messages in thread From: Lucas De Marchi @ 2024-09-11 16:18 UTC (permalink / raw) To: Bommu, Krishnaiah Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Kamil Konieczny, Ceraolo Spurio, Daniele, Upadhyay, Tejas, Tvrtko Ursulin, Joonas Lahtinen, Nikula, Jani, Thomas Hellström, Teres Alexis, Alan Previn, Winkler, Tomas, Usyskin, Alexander, linux-modules, Luis Chamberlain + linux-modules + Luis On Wed, Sep 11, 2024 at 01:00:47AM GMT, Bommu, Krishnaiah wrote: > > >> -----Original Message----- >> From: De Marchi, Lucas <lucas.demarchi@intel.com> >> Sent: Tuesday, September 10, 2024 9:13 PM >> To: Vivi, Rodrigo <rodrigo.vivi@intel.com> >> Cc: Bommu, Krishnaiah <krishnaiah.bommu@intel.com>; intel- >> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kamil Konieczny >> <kamil.konieczny@linux.intel.com>; Ceraolo Spurio, Daniele >> <daniele.ceraolospurio@intel.com>; Upadhyay, Tejas >> <tejas.upadhyay@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; Joonas >> Lahtinen <joonas.lahtinen@linux.intel.com>; Nikula, Jani >> <jani.nikula@intel.com>; Thomas Hellström >> <thomas.hellstrom@linux.intel.com>; Teres Alexis, Alan Previn >> <alan.previn.teres.alexis@intel.com>; Winkler, Tomas >> <tomas.winkler@intel.com>; Usyskin, Alexander >> <alexander.usyskin@intel.com> >> Subject: Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI >> Modules for i915/Xe Driver >> >> On Tue, Sep 10, 2024 at 11:03:30AM GMT, Rodrigo Vivi wrote: >> >On Mon, Sep 09, 2024 at 09:33:17AM +0530, Bommu Krishnaiah wrote: >> >> This update addresses the unload/reload sequence of MEI modules in >> >> relation to the i915/Xe graphics driver. On platforms where the MEI >> >> hardware is integrated with the graphics device (e.g., DG2/BMG), the >> >> i915/xe driver is depend on the MEI modules. Conversely, on newer >> >> platforms like MTL and LNL, where the MEI hardware is separate, this >> dependency does not exist. >> >> >> >> The changes introduced ensure that MEI modules are unloaded and >> >> reloaded in the correct order based on platform-specific >> >> dependencies. This is achieved by adding a MODULE_SOFTDEP directive to >> the i915 and Xe module code. >> >> >> can you explain what causes the modules to be loaded today? Also, is this to fix >> anything related to *loading* order or just unload? >> >> >> >> >> These changes enhance the robustness of MEI module handling across >> >> different hardware platforms, ensuring that the i915/Xe driver can be >> >> cleanly unloaded and reloaded without issues. >> >> >> >> v2: updated commit message >> >> >> >> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com> >> >> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> >> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> >> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/i915_module.c | 2 ++ >> >> drivers/gpu/drm/xe/xe_module.c | 2 ++ >> >> 2 files changed, 4 insertions(+) >> >> >> >> diff --git a/drivers/gpu/drm/i915/i915_module.c >> >> b/drivers/gpu/drm/i915/i915_module.c >> >> index 65acd7bf75d0..2ad079ad35db 100644 >> >> --- a/drivers/gpu/drm/i915/i915_module.c >> >> +++ b/drivers/gpu/drm/i915/i915_module.c >> >> @@ -75,6 +75,8 @@ static const struct { }; static int >> >> init_progress; >> >> >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); >> >> + >> >> static int __init i915_init(void) >> >> { >> >> int err, i; >> >> diff --git a/drivers/gpu/drm/xe/xe_module.c >> >> b/drivers/gpu/drm/xe/xe_module.c index bfc3deebdaa2..5633ea1841b7 >> >> 100644 >> >> --- a/drivers/gpu/drm/xe/xe_module.c >> >> +++ b/drivers/gpu/drm/xe/xe_module.c >> >> @@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i) >> >> init_funcs[i].exit(); >> >> } >> >> >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); >> > >> >I'm honestly not very comfortable with this. >> > >> >1. This is not true for every device supported by these modules. >> >2. This is not true for every (and the most basic) functionality of these drivers. >> > >> >Shouldn't this be done in the the mei side? >> >> I don't think it's possible to do from the mei side. Would mei depend on both xe >> and i915 (and thus cause both to be loaded regardless of the platform?). For a >> runtime dependency like this that depends on the platform, I think the best way >> would be a weakdep + either a request_module() or something else that causes >> the module to load (is that what comp_* is doing today?) >> >> > >> >Couldn't at probe we identify the need of them and if needed we return >> >-EPROBE to attempt a retry after the mei drivers were probed? >> >> I'm not sure this is fixing anything for probe. I think we already wait on the other >> component to be ready without blocking the rest of the driver functionality. >> >> A weakdep wouldn't cause the module to be loaded where it's not needed, but >> need some clarification if this is trying to fix anything load-related or just unload. > >This change is fixing unload. >During xe load I am seeing mei_gsc modules was loaded, but not unloaded during the unload xe so, first thing: if things are correct in the kernel, we shouldn't need to **unload** the module after unbinding the device. Why are we unloading xe and the other modules for tests? > >root@DUT6127BMGFRD:/home/gta# lsmod | grep xe ------>>>just after system reboot >root@DUT6127BMGFRD:/home/gta# >root@DUT6127BMGFRD:/home/gta# lsmod | grep mei >mei_hdcp 28672 0 >mei_pxp 16384 0 >mei_me 49152 2 >mei 167936 5 mei_hdcp,mei_pxp,mei_me >root@DUT6127BMGFRD:/home/gta# lsmod | grep xe >root@DUT6127BMGFRD:/home/gta# >root@DUT6127BMGFRD:/home/gta# modprobe xe >root@DUT6127BMGFRD:/home/gta# >root@DUT6127BMGFRD:/home/gta# lsmod | grep mei >mei_gsc_proxy 16384 0 >mei_gsc 12288 1 ^ which means there's one user, which should be xe >mei_hdcp 28672 0 >mei_pxp 16384 0 >mei_me 49152 3 mei_gsc >mei 167936 8 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me >root@DUT6127BMGFRD:/home/gta# >root@DUT6127BMGFRD:/home/gta# >root@DUT6127BMGFRD:/home/gta# >root@DUT6127BMGFRD:/home/gta# init 3 >root@DUT6127BMGFRD:/home/gta# echo -n auto > /sys/bus/pci/devices/0000\:03\:00.0/power/control >root@DUT6127BMGFRD:/home/gta# echo -n "0000:03:00.0" > /sys/bus/pci/drivers/xe/unbind >root@DUT6127BMGFRD:/home/gta# modprobe -r xe >root@DUT6127BMGFRD:/home/gta# >root@DUT6127BMGFRD:/home/gta# lsmod | grep xe >root@DUT6127BMGFRD:/home/gta# lsmod | grep mei >mei_gsc_proxy 16384 0 >mei_gsc 12288 0 ^ great, so the refcount went to 0, confirming it was xe. It should go to 0 even before you unload the module, when unbind. A couple of points: 1) why do we care about unloading mei_gsc. Just loading xe again (or even not even unloading it, just unbind/rebind), should still work if the xe <-> mei_gsc integration is done correctly. 2) If for some reason we do want to remove the module, then we will need some work in kernel/module/ to start tracking runtime module dependencies, i.e. when one module does a module_get(foo->owner), it would add to a list and output on sysfs together with the holders list. This way you would be able to track the runtime deps and remove them if their refcount went to 0 after removing xe. (2) is doable, but previous attempts were not successful [1]. Is there something else to make the simpler solution (1) to work? thanks Lucas De Marchi [1] https://lore.kernel.org/linux-modules/cover.1652113087.git.mchehab@kernel.org/ >mei_hdcp 28672 0 >mei_pxp 16384 0 >mei_me 49152 3 mei_gsc >mei 167936 7 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me >root@DUT6127BMGFRD:/home/gta# > >Regards, >Krishna. > >> >> Lucas De Marchi >> >> > >> >Cc: Alexander Usyskin <alexander.usyskin@intel.com> >> >Cc: Tomas Winkler <tomas.winkler@intel.com> >> >Cc: Alan Previn <alan.previn.teres.alexis@intel.com> >> >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> >Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >> >Cc: Jani Nikula <jani.nikula@intel.com> >> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> >Cc: Tvrtko Ursulin <tursulin@ursulin.net> >> > >> >> + >> >> static int __init xe_init(void) >> >> { >> >> int err, i; >> >> -- >> >> 2.25.1 >> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-11 16:18 ` Lucas De Marchi @ 2024-09-12 11:58 ` Bommu, Krishnaiah 2024-09-12 20:42 ` Lucas De Marchi 0 siblings, 1 reply; 11+ messages in thread From: Bommu, Krishnaiah @ 2024-09-12 11:58 UTC (permalink / raw) To: De Marchi, Lucas Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Kamil Konieczny, Ceraolo Spurio, Daniele, Upadhyay, Tejas, Tvrtko Ursulin, Joonas Lahtinen, Nikula, Jani, Thomas Hellström, Teres Alexis, Alan Previn, Winkler, Tomas, Usyskin, Alexander, linux-modules@vger.kernel.org, Luis Chamberlain > -----Original Message----- > From: De Marchi, Lucas <lucas.demarchi@intel.com> > Sent: Wednesday, September 11, 2024 9:49 PM > To: Bommu, Krishnaiah <krishnaiah.bommu@intel.com> > Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; intel-xe@lists.freedesktop.org; intel- > gfx@lists.freedesktop.org; Kamil Konieczny <kamil.konieczny@linux.intel.com>; > Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com>; Upadhyay, Tejas > <tejas.upadhyay@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; Joonas > Lahtinen <joonas.lahtinen@linux.intel.com>; Nikula, Jani > <jani.nikula@intel.com>; Thomas Hellström > <thomas.hellstrom@linux.intel.com>; Teres Alexis, Alan Previn > <alan.previn.teres.alexis@intel.com>; Winkler, Tomas > <tomas.winkler@intel.com>; Usyskin, Alexander > <alexander.usyskin@intel.com>; linux-modules@vger.kernel.org; Luis > Chamberlain <mcgrof@kernel.org> > Subject: Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI > Modules for i915/Xe Driver > > + linux-modules > + Luis > > On Wed, Sep 11, 2024 at 01:00:47AM GMT, Bommu, Krishnaiah wrote: > > > > > >> -----Original Message----- > >> From: De Marchi, Lucas <lucas.demarchi@intel.com> > >> Sent: Tuesday, September 10, 2024 9:13 PM > >> To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > >> Cc: Bommu, Krishnaiah <krishnaiah.bommu@intel.com>; intel- > >> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kamil > >> Konieczny <kamil.konieczny@linux.intel.com>; Ceraolo Spurio, Daniele > >> <daniele.ceraolospurio@intel.com>; Upadhyay, Tejas > >> <tejas.upadhyay@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; > >> Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Nikula, Jani > >> <jani.nikula@intel.com>; Thomas Hellström > >> <thomas.hellstrom@linux.intel.com>; Teres Alexis, Alan Previn > >> <alan.previn.teres.alexis@intel.com>; Winkler, Tomas > >> <tomas.winkler@intel.com>; Usyskin, Alexander > >> <alexander.usyskin@intel.com> > >> Subject: Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI > >> Modules for i915/Xe Driver > >> > >> On Tue, Sep 10, 2024 at 11:03:30AM GMT, Rodrigo Vivi wrote: > >> >On Mon, Sep 09, 2024 at 09:33:17AM +0530, Bommu Krishnaiah wrote: > >> >> This update addresses the unload/reload sequence of MEI modules in > >> >> relation to the i915/Xe graphics driver. On platforms where the > >> >> MEI hardware is integrated with the graphics device (e.g., > >> >> DG2/BMG), the i915/xe driver is depend on the MEI modules. > >> >> Conversely, on newer platforms like MTL and LNL, where the MEI > >> >> hardware is separate, this > >> dependency does not exist. > >> >> > >> >> The changes introduced ensure that MEI modules are unloaded and > >> >> reloaded in the correct order based on platform-specific > >> >> dependencies. This is achieved by adding a MODULE_SOFTDEP > >> >> directive to > >> the i915 and Xe module code. > >> > >> > >> can you explain what causes the modules to be loaded today? Also, is > >> this to fix anything related to *loading* order or just unload? > >> > >> >> > >> >> These changes enhance the robustness of MEI module handling across > >> >> different hardware platforms, ensuring that the i915/Xe driver can > >> >> be cleanly unloaded and reloaded without issues. > >> >> > >> >> v2: updated commit message > >> >> > >> >> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com> > >> >> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > >> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> > >> >> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > >> >> --- > >> >> drivers/gpu/drm/i915/i915_module.c | 2 ++ > >> >> drivers/gpu/drm/xe/xe_module.c | 2 ++ > >> >> 2 files changed, 4 insertions(+) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/i915_module.c > >> >> b/drivers/gpu/drm/i915/i915_module.c > >> >> index 65acd7bf75d0..2ad079ad35db 100644 > >> >> --- a/drivers/gpu/drm/i915/i915_module.c > >> >> +++ b/drivers/gpu/drm/i915/i915_module.c > >> >> @@ -75,6 +75,8 @@ static const struct { }; static int > >> >> init_progress; > >> >> > >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > >> >> + > >> >> static int __init i915_init(void) { > >> >> int err, i; > >> >> diff --git a/drivers/gpu/drm/xe/xe_module.c > >> >> b/drivers/gpu/drm/xe/xe_module.c index bfc3deebdaa2..5633ea1841b7 > >> >> 100644 > >> >> --- a/drivers/gpu/drm/xe/xe_module.c > >> >> +++ b/drivers/gpu/drm/xe/xe_module.c > >> >> @@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i) > >> >> init_funcs[i].exit(); > >> >> } > >> >> > >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > >> > > >> >I'm honestly not very comfortable with this. > >> > > >> >1. This is not true for every device supported by these modules. > >> >2. This is not true for every (and the most basic) functionality of these > drivers. > >> > > >> >Shouldn't this be done in the the mei side? > >> > >> I don't think it's possible to do from the mei side. Would mei depend > >> on both xe and i915 (and thus cause both to be loaded regardless of > >> the platform?). For a runtime dependency like this that depends on > >> the platform, I think the best way would be a weakdep + either a > >> request_module() or something else that causes the module to load (is > >> that what comp_* is doing today?) > >> > >> > > >> >Couldn't at probe we identify the need of them and if needed we > >> >return -EPROBE to attempt a retry after the mei drivers were probed? > >> > >> I'm not sure this is fixing anything for probe. I think we already > >> wait on the other component to be ready without blocking the rest of the > driver functionality. > >> > >> A weakdep wouldn't cause the module to be loaded where it's not > >> needed, but need some clarification if this is trying to fix anything load- > related or just unload. > > > >This change is fixing unload. > >During xe load I am seeing mei_gsc modules was loaded, but not unloaded > >during the unload xe > > so, first thing: if things are correct in the kernel, we shouldn't need to > **unload** the module after unbinding the device. Why are we unloading xe > and the other modules for tests? While running gta@xe_module_load@reload-no-display I see failure, to address this failure I have this changes, previously I am trying to fix from IGT, but as per igt review suggestion I am trying to fix issue in kernel, IGT patch: https://patchwork.freedesktop.org/series/137343/ > >root@DUT6127BMGFRD:/home/gta# lsmod | grep xe ------>>>just after > >system reboot root@DUT6127BMGFRD:/home/gta# > >root@DUT6127BMGFRD:/home/gta# lsmod | grep mei > >mei_hdcp 28672 0 > >mei_pxp 16384 0 > >mei_me 49152 2 > >mei 167936 5 mei_hdcp,mei_pxp,mei_me > >root@DUT6127BMGFRD:/home/gta# lsmod | grep xe > >root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# > modprobe xe > >root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# > lsmod | > >grep mei > >mei_gsc_proxy 16384 0 > >mei_gsc 12288 1 > > ^ which means there's one user, which > should be xe > > >mei_hdcp 28672 0 > >mei_pxp 16384 0 > >mei_me 49152 3 mei_gsc > >mei 167936 8 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me > >root@DUT6127BMGFRD:/home/gta# > >root@DUT6127BMGFRD:/home/gta# > >root@DUT6127BMGFRD:/home/gta# > >root@DUT6127BMGFRD:/home/gta# init 3 > >root@DUT6127BMGFRD:/home/gta# echo -n auto > > >/sys/bus/pci/devices/0000\:03\:00.0/power/control > >root@DUT6127BMGFRD:/home/gta# echo -n "0000:03:00.0" > > >/sys/bus/pci/drivers/xe/unbind root@DUT6127BMGFRD:/home/gta# > modprobe > >-r xe root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# lsmod > >| grep xe root@DUT6127BMGFRD:/home/gta# lsmod | grep mei > >mei_gsc_proxy 16384 0 > >mei_gsc 12288 0 > > ^ great, so the refcount went to 0, > confirming it was xe. It should go to 0 > even before you unload the module, > when unbind. > > A couple of points: > > 1) why do we care about unloading mei_gsc. Just loading xe > again (or even not even unloading it, just unbind/rebind), > should still work if the xe <-> mei_gsc integration is done > correctly. > > 2) If for some reason we do want to remove the module, then we will > need some work in kernel/module/ to start tracking runtime module > dependencies, i.e. when one module does a module_get(foo->owner), it > would add to a list and output on sysfs together with the holders list. > This way you would be able to track the runtime deps and remove them > if their refcount went to 0 after removing xe. > > (2) is doable, but previous attempts were not successful [1]. Is there something > else to make the simpler solution (1) to work? > Reference why I am doing this changes, please see review comments of this patch https://patchwork.freedesktop.org/series/137343/ Regards, Krishna. > thanks > Lucas De Marchi > > [1] https://lore.kernel.org/linux- > modules/cover.1652113087.git.mchehab@kernel.org/ > > >mei_hdcp 28672 0 > >mei_pxp 16384 0 > >mei_me 49152 3 mei_gsc > >mei 167936 7 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me > >root@DUT6127BMGFRD:/home/gta# > > > >Regards, > >Krishna. > > > >> > >> Lucas De Marchi > >> > >> > > >> >Cc: Alexander Usyskin <alexander.usyskin@intel.com> > >> >Cc: Tomas Winkler <tomas.winkler@intel.com> > >> >Cc: Alan Previn <alan.previn.teres.alexis@intel.com> > >> >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> > >> >Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > >> >Cc: Jani Nikula <jani.nikula@intel.com> > >> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > >> >Cc: Tvrtko Ursulin <tursulin@ursulin.net> > >> > > >> >> + > >> >> static int __init xe_init(void) > >> >> { > >> >> int err, i; > >> >> -- > >> >> 2.25.1 > >> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-12 11:58 ` Bommu, Krishnaiah @ 2024-09-12 20:42 ` Lucas De Marchi 2024-09-13 22:21 ` Lucas De Marchi 0 siblings, 1 reply; 11+ messages in thread From: Lucas De Marchi @ 2024-09-12 20:42 UTC (permalink / raw) To: Bommu, Krishnaiah Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Kamil Konieczny, Ceraolo Spurio, Daniele, Upadhyay, Tejas, Tvrtko Ursulin, Joonas Lahtinen, Nikula, Jani, Thomas Hellström, Teres Alexis, Alan Previn, Winkler, Tomas, Usyskin, Alexander, linux-modules@vger.kernel.org, Luis Chamberlain On Thu, Sep 12, 2024 at 11:58:37AM GMT, Bommu, Krishnaiah wrote: > > >> -----Original Message----- >> From: De Marchi, Lucas <lucas.demarchi@intel.com> >> Sent: Wednesday, September 11, 2024 9:49 PM >> To: Bommu, Krishnaiah <krishnaiah.bommu@intel.com> >> Cc: Vivi, Rodrigo <rodrigo.vivi@intel.com>; intel-xe@lists.freedesktop.org; intel- >> gfx@lists.freedesktop.org; Kamil Konieczny <kamil.konieczny@linux.intel.com>; >> Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com>; Upadhyay, Tejas >> <tejas.upadhyay@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; Joonas >> Lahtinen <joonas.lahtinen@linux.intel.com>; Nikula, Jani >> <jani.nikula@intel.com>; Thomas Hellström >> <thomas.hellstrom@linux.intel.com>; Teres Alexis, Alan Previn >> <alan.previn.teres.alexis@intel.com>; Winkler, Tomas >> <tomas.winkler@intel.com>; Usyskin, Alexander >> <alexander.usyskin@intel.com>; linux-modules@vger.kernel.org; Luis >> Chamberlain <mcgrof@kernel.org> >> Subject: Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI >> Modules for i915/Xe Driver >> >> + linux-modules >> + Luis >> >> On Wed, Sep 11, 2024 at 01:00:47AM GMT, Bommu, Krishnaiah wrote: >> > >> > >> >> -----Original Message----- >> >> From: De Marchi, Lucas <lucas.demarchi@intel.com> >> >> Sent: Tuesday, September 10, 2024 9:13 PM >> >> To: Vivi, Rodrigo <rodrigo.vivi@intel.com> >> >> Cc: Bommu, Krishnaiah <krishnaiah.bommu@intel.com>; intel- >> >> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kamil >> >> Konieczny <kamil.konieczny@linux.intel.com>; Ceraolo Spurio, Daniele >> >> <daniele.ceraolospurio@intel.com>; Upadhyay, Tejas >> >> <tejas.upadhyay@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; >> >> Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Nikula, Jani >> >> <jani.nikula@intel.com>; Thomas Hellström >> >> <thomas.hellstrom@linux.intel.com>; Teres Alexis, Alan Previn >> >> <alan.previn.teres.alexis@intel.com>; Winkler, Tomas >> >> <tomas.winkler@intel.com>; Usyskin, Alexander >> >> <alexander.usyskin@intel.com> >> >> Subject: Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI >> >> Modules for i915/Xe Driver >> >> >> >> On Tue, Sep 10, 2024 at 11:03:30AM GMT, Rodrigo Vivi wrote: >> >> >On Mon, Sep 09, 2024 at 09:33:17AM +0530, Bommu Krishnaiah wrote: >> >> >> This update addresses the unload/reload sequence of MEI modules in >> >> >> relation to the i915/Xe graphics driver. On platforms where the >> >> >> MEI hardware is integrated with the graphics device (e.g., >> >> >> DG2/BMG), the i915/xe driver is depend on the MEI modules. >> >> >> Conversely, on newer platforms like MTL and LNL, where the MEI >> >> >> hardware is separate, this >> >> dependency does not exist. >> >> >> >> >> >> The changes introduced ensure that MEI modules are unloaded and >> >> >> reloaded in the correct order based on platform-specific >> >> >> dependencies. This is achieved by adding a MODULE_SOFTDEP >> >> >> directive to >> >> the i915 and Xe module code. >> >> >> >> >> >> can you explain what causes the modules to be loaded today? Also, is >> >> this to fix anything related to *loading* order or just unload? >> >> >> >> >> >> >> >> These changes enhance the robustness of MEI module handling across >> >> >> different hardware platforms, ensuring that the i915/Xe driver can >> >> >> be cleanly unloaded and reloaded without issues. >> >> >> >> >> >> v2: updated commit message >> >> >> >> >> >> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com> >> >> >> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> >> >> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> >> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> >> >> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> >> >> >> --- >> >> >> drivers/gpu/drm/i915/i915_module.c | 2 ++ >> >> >> drivers/gpu/drm/xe/xe_module.c | 2 ++ >> >> >> 2 files changed, 4 insertions(+) >> >> >> >> >> >> diff --git a/drivers/gpu/drm/i915/i915_module.c >> >> >> b/drivers/gpu/drm/i915/i915_module.c >> >> >> index 65acd7bf75d0..2ad079ad35db 100644 >> >> >> --- a/drivers/gpu/drm/i915/i915_module.c >> >> >> +++ b/drivers/gpu/drm/i915/i915_module.c >> >> >> @@ -75,6 +75,8 @@ static const struct { }; static int >> >> >> init_progress; >> >> >> >> >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); >> >> >> + >> >> >> static int __init i915_init(void) { >> >> >> int err, i; >> >> >> diff --git a/drivers/gpu/drm/xe/xe_module.c >> >> >> b/drivers/gpu/drm/xe/xe_module.c index bfc3deebdaa2..5633ea1841b7 >> >> >> 100644 >> >> >> --- a/drivers/gpu/drm/xe/xe_module.c >> >> >> +++ b/drivers/gpu/drm/xe/xe_module.c >> >> >> @@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i) >> >> >> init_funcs[i].exit(); >> >> >> } >> >> >> >> >> >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); >> >> > >> >> >I'm honestly not very comfortable with this. >> >> > >> >> >1. This is not true for every device supported by these modules. >> >> >2. This is not true for every (and the most basic) functionality of these >> drivers. >> >> > >> >> >Shouldn't this be done in the the mei side? >> >> >> >> I don't think it's possible to do from the mei side. Would mei depend >> >> on both xe and i915 (and thus cause both to be loaded regardless of >> >> the platform?). For a runtime dependency like this that depends on >> >> the platform, I think the best way would be a weakdep + either a >> >> request_module() or something else that causes the module to load (is >> >> that what comp_* is doing today?) >> >> >> >> > >> >> >Couldn't at probe we identify the need of them and if needed we >> >> >return -EPROBE to attempt a retry after the mei drivers were probed? >> >> >> >> I'm not sure this is fixing anything for probe. I think we already >> >> wait on the other component to be ready without blocking the rest of the >> driver functionality. >> >> >> >> A weakdep wouldn't cause the module to be loaded where it's not >> >> needed, but need some clarification if this is trying to fix anything load- >> related or just unload. >> > >> >This change is fixing unload. >> >During xe load I am seeing mei_gsc modules was loaded, but not unloaded >> >during the unload xe >> >> so, first thing: if things are correct in the kernel, we shouldn't need to >> **unload** the module after unbinding the device. Why are we unloading xe >> and the other modules for tests? > >While running gta@xe_module_load@reload-no-display I see failure, to address this failure I have this changes, previously I am trying to fix from IGT, but as per igt review suggestion I am trying to fix issue in kernel, >IGT patch: https://patchwork.freedesktop.org/series/137343/ it seems a mistake in igt to try to remove the mei_gsc module. As a dgfx, it's even worse - what happens if another card is using the module? What happens if I have a RPL + BMG and i915 driving the former while xe drives the latter? You shouldn't need to remove it. This works for me with BMG (unbinding all drivers for simplicity since we are removing the module... but if we don't remove the module, then we can test with the only device we care about): # modprobe xe # unbind Unbinding /sys/bus/pci/devices/0000:00:02.0 (8086:a782)... ok Unbinding /sys/bus/pci/devices/0000:03:00.0 (8086:e20b)... ok # lsmod | grep -e xe -e mei_gsc xe 3584000 0 drm_gpuvm 45056 1 xe video 77824 1 xe i2c_algo_bit 12288 1 xe drm_ttm_helper 16384 1 xe gpu_sched 61440 1 xe drm_suballoc_helper 16384 1 xe drm_display_helper 270336 1 xe drm_kunit_helpers 16384 1 xe drm_buddy 20480 1 xe ttm 114688 2 drm_ttm_helper,xe mei_gsc_proxy 16384 0 mei_gsc 12288 0 drm_exec 16384 2 drm_gpuvm,xe kunit 73728 2 xe,drm_kunit_helpers drm_kms_helper 241664 4 drm_display_helper,drm_ttm_helper,xe,drm_kunit_helpers mei_me 65536 3 mei_gsc mei 167936 7 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me drm 737280 11 gpu_sched,drm_kms_helper,drm_exec,drm_gpuvm,drm_suballoc_helper,drm_display_helper,drm_buddy,drm_ttm_helper,xe,drm_kunit_helpers,ttm # modprobe -r xe # modprobe xe probe_display=0 # unbind Unbinding /sys/bus/pci/devices/0000:00:02.0 (8086:a782)... ok Unbinding /sys/bus/pci/devices/0000:03:00.0 (8086:e20b)... ok # modprobe -r xe # modprobe xe I didn't check if mei_gsc continues to work after reload, but I guess so as its refcount is incremented: mei_gsc 12288 1 unbind function is this: function unbind { vga="0300" display="0380" pci_vendor="8086" while read -r pci_slot class devid xxx; do sysdev=/sys/bus/pci/devices/0000:$pci_slot echo -n "Unbinding $sysdev ($devid)... " if [ ! -e "$sysdev/driver" ]; then echo "(skip: not bound)" continue fi echo -n auto > ${sysdev}/power/control echo -n "0000:$pci_slot" > $sysdev/driver/unbind echo "ok" done <<<$(lspci -d ${pci_vendor}::${display} -n; lspci -d ${pci_vendor}::${vga} -n ) } So... for igt: I *think* simply removing the array with modules to unload first would fix it. Lucas De Marchi > >> >root@DUT6127BMGFRD:/home/gta# lsmod | grep xe ------>>>just after >> >system reboot root@DUT6127BMGFRD:/home/gta# >> >root@DUT6127BMGFRD:/home/gta# lsmod | grep mei >> >mei_hdcp 28672 0 >> >mei_pxp 16384 0 >> >mei_me 49152 2 >> >mei 167936 5 mei_hdcp,mei_pxp,mei_me >> >root@DUT6127BMGFRD:/home/gta# lsmod | grep xe >> >root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# >> modprobe xe >> >root@DUT6127BMGFRD:/home/gta# root@DUT6127BMGFRD:/home/gta# >> lsmod | >> >grep mei >> >mei_gsc_proxy 16384 0 >> >mei_gsc 12288 1 >> >> ^ which means there's one user, which >> should be xe >> >> >mei_hdcp 28672 0 >> >mei_pxp 16384 0 >> >mei_me 49152 3 mei_gsc >> >mei 167936 8 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me >> >root@DUT6127BMGFRD:/home/gta# >> >root@DUT6127BMGFRD:/home/gta# >> >root@DUT6127BMGFRD:/home/gta# >> >root@DUT6127BMGFRD:/home/gta# init 3 >> >root@DUT6127BMGFRD:/home/gta# echo -n auto > >> >/sys/bus/pci/devices/0000\:03\:00.0/power/control >> >root@DUT6127BMGFRD:/home/gta# echo -n "0000:03:00.0" > >> >/sys/bus/pci/drivers/xe/unbind root@DUT6127BMGFRD:/home/gta# >> modprobe >> >-r xe root@DUT6127BMGFRD:/home/gta# >> root@DUT6127BMGFRD:/home/gta# lsmod >> >| grep xe root@DUT6127BMGFRD:/home/gta# lsmod | grep mei >> >mei_gsc_proxy 16384 0 >> >mei_gsc 12288 0 >> >> ^ great, so the refcount went to 0, >> confirming it was xe. It should go to 0 >> even before you unload the module, >> when unbind. >> >> A couple of points: >> >> 1) why do we care about unloading mei_gsc. Just loading xe >> again (or even not even unloading it, just unbind/rebind), >> should still work if the xe <-> mei_gsc integration is done >> correctly. >> >> 2) If for some reason we do want to remove the module, then we will >> need some work in kernel/module/ to start tracking runtime module >> dependencies, i.e. when one module does a module_get(foo->owner), it >> would add to a list and output on sysfs together with the holders list. >> This way you would be able to track the runtime deps and remove them >> if their refcount went to 0 after removing xe. >> >> (2) is doable, but previous attempts were not successful [1]. Is there something >> else to make the simpler solution (1) to work? >> > >Reference why I am doing this changes, please see review comments of this patch https://patchwork.freedesktop.org/series/137343/ > >Regards, >Krishna. > >> thanks >> Lucas De Marchi >> >> [1] https://lore.kernel.org/linux- >> modules/cover.1652113087.git.mchehab@kernel.org/ >> >> >mei_hdcp 28672 0 >> >mei_pxp 16384 0 >> >mei_me 49152 3 mei_gsc >> >mei 167936 7 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me >> >root@DUT6127BMGFRD:/home/gta# >> > >> >Regards, >> >Krishna. >> > >> >> >> >> Lucas De Marchi >> >> >> >> > >> >> >Cc: Alexander Usyskin <alexander.usyskin@intel.com> >> >> >Cc: Tomas Winkler <tomas.winkler@intel.com> >> >> >Cc: Alan Previn <alan.previn.teres.alexis@intel.com> >> >> >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >> >> >Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >> >> >Cc: Jani Nikula <jani.nikula@intel.com> >> >> >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> >> >Cc: Tvrtko Ursulin <tursulin@ursulin.net> >> >> > >> >> >> + >> >> >> static int __init xe_init(void) >> >> >> { >> >> >> int err, i; >> >> >> -- >> >> >> 2.25.1 >> >> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-12 20:42 ` Lucas De Marchi @ 2024-09-13 22:21 ` Lucas De Marchi 0 siblings, 0 replies; 11+ messages in thread From: Lucas De Marchi @ 2024-09-13 22:21 UTC (permalink / raw) To: Bommu, Krishnaiah Cc: Vivi, Rodrigo, intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Kamil Konieczny, Ceraolo Spurio, Daniele, Upadhyay, Tejas, Tvrtko Ursulin, Joonas Lahtinen, Nikula, Jani, Thomas Hellström, Teres Alexis, Alan Previn, Winkler, Tomas, Usyskin, Alexander, linux-modules@vger.kernel.org, Luis Chamberlain On Thu, Sep 12, 2024 at 03:42:52PM GMT, Lucas De Marchi wrote: >unbind function is this: > >function unbind { > vga="0300" > display="0380" > pci_vendor="8086" > > while read -r pci_slot class devid xxx; do > sysdev=/sys/bus/pci/devices/0000:$pci_slot > > echo -n "Unbinding $sysdev ($devid)... " > if [ ! -e "$sysdev/driver" ]; then > echo "(skip: not bound)" > continue > fi > > echo -n auto > ${sysdev}/power/control > echo -n "0000:$pci_slot" > $sysdev/driver/unbind > echo "ok" > done <<<$(lspci -d ${pci_vendor}::${display} -n; lspci -d ${pci_vendor}::${vga} -n ) >} > > >So... for igt: I *think* simply removing the array with modules to >unload first would fix it. I decided to be more useful than just giving the sketch above and typed something similar to what I'm writing for kmod (soon we will have `kmod [bind|unbind]` commands): https://patchwork.freedesktop.org/series/138676/ xe_module_load@reload-no-display works for me with BMG with that patch. Let's see if it passes the rest of the CI tests. Lucas De Marchi ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver 2024-09-11 6:00 ` Bommu, Krishnaiah 2024-09-11 16:18 ` Lucas De Marchi @ 2024-09-11 20:41 ` Rodrigo Vivi 1 sibling, 0 replies; 11+ messages in thread From: Rodrigo Vivi @ 2024-09-11 20:41 UTC (permalink / raw) To: Bommu, Krishnaiah Cc: De Marchi, Lucas, intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Kamil Konieczny, Ceraolo Spurio, Daniele, Upadhyay, Tejas, Tvrtko Ursulin, Joonas Lahtinen, Nikula, Jani, Thomas Hellström, Teres Alexis, Alan Previn, Winkler, Tomas, Usyskin, Alexander On Wed, Sep 11, 2024 at 06:00:47AM +0000, Bommu, Krishnaiah wrote: > > > > -----Original Message----- > > From: De Marchi, Lucas <lucas.demarchi@intel.com> > > Sent: Tuesday, September 10, 2024 9:13 PM > > To: Vivi, Rodrigo <rodrigo.vivi@intel.com> > > Cc: Bommu, Krishnaiah <krishnaiah.bommu@intel.com>; intel- > > xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kamil Konieczny > > <kamil.konieczny@linux.intel.com>; Ceraolo Spurio, Daniele > > <daniele.ceraolospurio@intel.com>; Upadhyay, Tejas > > <tejas.upadhyay@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; Joonas > > Lahtinen <joonas.lahtinen@linux.intel.com>; Nikula, Jani > > <jani.nikula@intel.com>; Thomas Hellström > > <thomas.hellstrom@linux.intel.com>; Teres Alexis, Alan Previn > > <alan.previn.teres.alexis@intel.com>; Winkler, Tomas > > <tomas.winkler@intel.com>; Usyskin, Alexander > > <alexander.usyskin@intel.com> > > Subject: Re: [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI > > Modules for i915/Xe Driver > > > > On Tue, Sep 10, 2024 at 11:03:30AM GMT, Rodrigo Vivi wrote: > > >On Mon, Sep 09, 2024 at 09:33:17AM +0530, Bommu Krishnaiah wrote: > > >> This update addresses the unload/reload sequence of MEI modules in > > >> relation to the i915/Xe graphics driver. On platforms where the MEI > > >> hardware is integrated with the graphics device (e.g., DG2/BMG), the > > >> i915/xe driver is depend on the MEI modules. Conversely, on newer > > >> platforms like MTL and LNL, where the MEI hardware is separate, this > > dependency does not exist. > > >> > > >> The changes introduced ensure that MEI modules are unloaded and > > >> reloaded in the correct order based on platform-specific > > >> dependencies. This is achieved by adding a MODULE_SOFTDEP directive to > > the i915 and Xe module code. > > > > > > can you explain what causes the modules to be loaded today? Also, is this to fix > > anything related to *loading* order or just unload? > > > > >> > > >> These changes enhance the robustness of MEI module handling across > > >> different hardware platforms, ensuring that the i915/Xe driver can be > > >> cleanly unloaded and reloaded without issues. > > >> > > >> v2: updated commit message > > >> > > >> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com> > > >> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > > >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > >> Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > >> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com> > > >> --- > > >> drivers/gpu/drm/i915/i915_module.c | 2 ++ > > >> drivers/gpu/drm/xe/xe_module.c | 2 ++ > > >> 2 files changed, 4 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/i915/i915_module.c > > >> b/drivers/gpu/drm/i915/i915_module.c > > >> index 65acd7bf75d0..2ad079ad35db 100644 > > >> --- a/drivers/gpu/drm/i915/i915_module.c > > >> +++ b/drivers/gpu/drm/i915/i915_module.c > > >> @@ -75,6 +75,8 @@ static const struct { }; static int > > >> init_progress; > > >> > > >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > > >> + > > >> static int __init i915_init(void) > > >> { > > >> int err, i; > > >> diff --git a/drivers/gpu/drm/xe/xe_module.c > > >> b/drivers/gpu/drm/xe/xe_module.c index bfc3deebdaa2..5633ea1841b7 > > >> 100644 > > >> --- a/drivers/gpu/drm/xe/xe_module.c > > >> +++ b/drivers/gpu/drm/xe/xe_module.c > > >> @@ -127,6 +127,8 @@ static void xe_call_exit_func(unsigned int i) > > >> init_funcs[i].exit(); > > >> } > > >> > > >> +MODULE_SOFTDEP("pre: mei_gsc_proxy mei_gsc"); > > > > > >I'm honestly not very comfortable with this. > > > > > >1. This is not true for every device supported by these modules. > > >2. This is not true for every (and the most basic) functionality of these drivers. > > > > > >Shouldn't this be done in the the mei side? > > > > I don't think it's possible to do from the mei side. Would mei depend on both xe > > and i915 (and thus cause both to be loaded regardless of the platform?). For a > > runtime dependency like this that depends on the platform, I think the best way > > would be a weakdep + either a request_module() or something else that causes > > the module to load (is that what comp_* is doing today?) > > > > > > > >Couldn't at probe we identify the need of them and if needed we return > > >-EPROBE to attempt a retry after the mei drivers were probed? > > > > I'm not sure this is fixing anything for probe. I think we already wait on the other > > component to be ready without blocking the rest of the driver functionality. > > > > A weakdep wouldn't cause the module to be loaded where it's not needed, but > > need some clarification if this is trying to fix anything load-related or just unload. > > This change is fixing unload. > During xe load I am seeing mei_gsc modules was loaded, but not unloaded during the unload xe Is it a problem? > > root@DUT6127BMGFRD:/home/gta# lsmod | grep xe ------>>>just after system reboot > root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# lsmod | grep mei > mei_hdcp 28672 0 > mei_pxp 16384 0 > mei_me 49152 2 > mei 167936 5 mei_hdcp,mei_pxp,mei_me > root@DUT6127BMGFRD:/home/gta# lsmod | grep xe > root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# modprobe xe > root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# lsmod | grep mei > mei_gsc_proxy 16384 0 > mei_gsc 12288 1 > mei_hdcp 28672 0 > mei_pxp 16384 0 > mei_me 49152 3 mei_gsc > mei 167936 8 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me > root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# init 3 > root@DUT6127BMGFRD:/home/gta# echo -n auto > /sys/bus/pci/devices/0000\:03\:00.0/power/control > root@DUT6127BMGFRD:/home/gta# echo -n "0000:03:00.0" > /sys/bus/pci/drivers/xe/unbind > root@DUT6127BMGFRD:/home/gta# modprobe -r xe > root@DUT6127BMGFRD:/home/gta# > root@DUT6127BMGFRD:/home/gta# lsmod | grep xe > root@DUT6127BMGFRD:/home/gta# lsmod | grep mei > mei_gsc_proxy 16384 0 > mei_gsc 12288 0 > mei_hdcp 28672 0 > mei_pxp 16384 0 > mei_me 49152 3 mei_gsc > mei 167936 7 mei_gsc_proxy,mei_gsc,mei_hdcp,mei_pxp,mei_me > root@DUT6127BMGFRD:/home/gta# > > Regards, > Krishna. > > > > > Lucas De Marchi > > > > > > > >Cc: Alexander Usyskin <alexander.usyskin@intel.com> > > >Cc: Tomas Winkler <tomas.winkler@intel.com> > > >Cc: Alan Previn <alan.previn.teres.alexis@intel.com> > > >Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > >Cc: Lucas De Marchi <lucas.demarchi@intel.com> > > >Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > > >Cc: Jani Nikula <jani.nikula@intel.com> > > >Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > >Cc: Tvrtko Ursulin <tursulin@ursulin.net> > > > > > >> + > > >> static int __init xe_init(void) > > >> { > > >> int err, i; > > >> -- > > >> 2.25.1 > > >> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-09-13 22:21 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-09 4:03 [PATCH v2] drm: Ensure Proper Unload/Reload Order of MEI Modules for i915/Xe Driver Bommu Krishnaiah 2024-09-09 4:50 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork 2024-09-09 5:11 ` ✗ Fi.CI.BAT: failure " Patchwork 2024-09-10 15:03 ` [PATCH v2] " Rodrigo Vivi 2024-09-10 15:43 ` Lucas De Marchi 2024-09-11 6:00 ` Bommu, Krishnaiah 2024-09-11 16:18 ` Lucas De Marchi 2024-09-12 11:58 ` Bommu, Krishnaiah 2024-09-12 20:42 ` Lucas De Marchi 2024-09-13 22:21 ` Lucas De Marchi 2024-09-11 20:41 ` Rodrigo Vivi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox