| Message ID | 20210719083534.2949348-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Check for nomodeset in i915_init() first | expand |
On 19/07/2021 09:35, Daniel Vetter wrote: > Jason is trying to sort out the unwinding in i915_init and i915_exit, > while reviewing those patches I noticed that we also have the > nomodeset handling now in the middle of things. > > Pull that out for simplisity in unwinding - if you run selftest with > nomodeset you get nothing, *shrug*. Where "nothing" equals success!? Quite bad for platform bringup on two levels. Regards, Tvrtko > Cc: Jason Ekstrand <jason@jlekstrand.net> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/i915_pci.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 67696d752271..6fe709ac1b4b 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -1199,14 +1199,6 @@ static int __init i915_init(void) > bool use_kms = true; > int err; > > - err = i915_globals_init(); > - if (err) > - return err; > - > - err = i915_mock_selftests(); > - if (err) > - return err > 0 ? 0 : err; > - > /* > * Enable KMS by default, unless explicitly overriden by > * either the i915.modeset prarameter or by the > @@ -1225,6 +1217,14 @@ static int __init i915_init(void) > return 0; > } > > + err = i915_globals_init(); > + if (err) > + return err; > + > + err = i915_mock_selftests(); > + if (err) > + return err > 0 ? 0 : err; > + > i915_pmu_init(); > > err = pci_register_driver(&i915_pci_driver); >
On 19/07/2021 10:09, Tvrtko Ursulin wrote: > > On 19/07/2021 09:35, Daniel Vetter wrote: >> Jason is trying to sort out the unwinding in i915_init and i915_exit, >> while reviewing those patches I noticed that we also have the >> nomodeset handling now in the middle of things. >> >> Pull that out for simplisity in unwinding - if you run selftest with >> nomodeset you get nothing, *shrug*. > > Where "nothing" equals success!? Quite bad for platform bringup on two > levels. Sorry I confused it with i915.disable_display. I am not aware nomodeset is actually used anywhere useful so my objection does not hold. Regards, Tvrtko > Regards, > > Tvrtko > >> Cc: Jason Ekstrand <jason@jlekstrand.net> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> drivers/gpu/drm/i915/i915_pci.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_pci.c >> b/drivers/gpu/drm/i915/i915_pci.c >> index 67696d752271..6fe709ac1b4b 100644 >> --- a/drivers/gpu/drm/i915/i915_pci.c >> +++ b/drivers/gpu/drm/i915/i915_pci.c >> @@ -1199,14 +1199,6 @@ static int __init i915_init(void) >> bool use_kms = true; >> int err; >> - err = i915_globals_init(); >> - if (err) >> - return err; >> - >> - err = i915_mock_selftests(); >> - if (err) >> - return err > 0 ? 0 : err; >> - >> /* >> * Enable KMS by default, unless explicitly overriden by >> * either the i915.modeset prarameter or by the >> @@ -1225,6 +1217,14 @@ static int __init i915_init(void) >> return 0; >> } >> + err = i915_globals_init(); >> + if (err) >> + return err; >> + >> + err = i915_mock_selftests(); >> + if (err) >> + return err > 0 ? 0 : err; >> + >> i915_pmu_init(); >> err = pci_register_driver(&i915_pci_driver); >>
On Mon, Jul 19, 2021 at 3:35 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Jason is trying to sort out the unwinding in i915_init and i915_exit, > while reviewing those patches I noticed that we also have the > nomodeset handling now in the middle of things. > > Pull that out for simplisity in unwinding - if you run selftest with > nomodeset you get nothing, *shrug*. > > Cc: Jason Ekstrand <jason@jlekstrand.net> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/i915_pci.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 67696d752271..6fe709ac1b4b 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -1199,14 +1199,6 @@ static int __init i915_init(void) > bool use_kms = true; > int err; > > - err = i915_globals_init(); > - if (err) > - return err; > - > - err = i915_mock_selftests(); > - if (err) > - return err > 0 ? 0 : err; > - > /* > * Enable KMS by default, unless explicitly overriden by > * either the i915.modeset prarameter or by the > @@ -1225,6 +1217,14 @@ static int __init i915_init(void) > return 0; > } > > + err = i915_globals_init(); > + if (err) > + return err; > + > + err = i915_mock_selftests(); > + if (err) > + return err > 0 ? 0 : err; > + Annoyingly, this actually makes i915_exit() harder because now we need to conditionals: One for "do you have globals?" and one for "do you have anything at all?". It's actually easier to get right if we have i915_globals_init() /* Everything that can return 0 early */ fully_loaded = true /* Everything that can fail */ > i915_pmu_init(); > > err = pci_register_driver(&i915_pci_driver); > -- > 2.32.0 >
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 67696d752271..6fe709ac1b4b 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1199,14 +1199,6 @@ static int __init i915_init(void) bool use_kms = true; int err; - err = i915_globals_init(); - if (err) - return err; - - err = i915_mock_selftests(); - if (err) - return err > 0 ? 0 : err; - /* * Enable KMS by default, unless explicitly overriden by * either the i915.modeset prarameter or by the @@ -1225,6 +1217,14 @@ static int __init i915_init(void) return 0; } + err = i915_globals_init(); + if (err) + return err; + + err = i915_mock_selftests(); + if (err) + return err > 0 ? 0 : err; + i915_pmu_init(); err = pci_register_driver(&i915_pci_driver);
Jason is trying to sort out the unwinding in i915_init and i915_exit, while reviewing those patches I noticed that we also have the nomodeset handling now in the middle of things. Pull that out for simplisity in unwinding - if you run selftest with nomodeset you get nothing, *shrug*. Cc: Jason Ekstrand <jason@jlekstrand.net> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/i915_pci.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)