| Message ID | 20230912120537.2158209-3-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: move more of the display probe to display code | expand |
On Tue, Sep 12, 2023 at 03:05:37PM +0300, Jani Nikula wrote: > Don't hide display probe in device info code. > > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_driver.c | 2 ++ > drivers/gpu/drm/i915/intel_device_info.c | 2 -- > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > 3 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index f8dbee7a5af7..b5e1f72cc3ce 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -735,6 +735,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) > /* Set up device info and initial runtime info. */ > intel_device_info_driver_create(i915, pdev->device, match_info); > > + intel_display_device_probe(i915); > + what about moving it even upper to i915_driver_probe? It would even make more sense with the name of the function... but up to you, this is already an improvement: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > return i915; > } > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index b9b8b22540cb..d2ed0f057cb2 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -418,8 +418,6 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, > runtime = RUNTIME_INFO(i915); > memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); > > - intel_display_device_probe(i915); > - > runtime->device_id = device_id; > } > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > index da0b269606c5..7de6477803f8 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) > /* Set up device info and initial runtime info. */ > intel_device_info_driver_create(i915, pdev->device, &mock_info); > > + intel_display_device_probe(i915); > + > dev_pm_domain_set(&pdev->dev, &pm_domain); > pm_runtime_enable(&pdev->dev); > pm_runtime_dont_use_autosuspend(&pdev->dev); > -- > 2.39.2 >
On Tue, Sep 12, 2023 at 03:05:37PM +0300, Jani Nikula wrote: > Don't hide display probe in device info code. > > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_driver.c | 2 ++ > drivers/gpu/drm/i915/intel_device_info.c | 2 -- > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > 3 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index f8dbee7a5af7..b5e1f72cc3ce 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -735,6 +735,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) > /* Set up device info and initial runtime info. */ > intel_device_info_driver_create(i915, pdev->device, match_info); > > + intel_display_device_probe(i915); > + > return i915; > } > > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index b9b8b22540cb..d2ed0f057cb2 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -418,8 +418,6 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, > runtime = RUNTIME_INFO(i915); > memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); > > - intel_display_device_probe(i915); > - > runtime->device_id = device_id; > } > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > index da0b269606c5..7de6477803f8 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) > /* Set up device info and initial runtime info. */ > intel_device_info_driver_create(i915, pdev->device, &mock_info); > > + intel_display_device_probe(i915); > + I realize this call was already happening with the old code, but do we really want to be probing the (real) display hardware while setting up mock (fake hardware) selftests? I don't think any of the mock tests should be doing anything display-related, and if they did we'd probably want to add some kind of mock_display_device_probe() instead or using the real hardware? Matt > dev_pm_domain_set(&pdev->dev, &pm_domain); > pm_runtime_enable(&pdev->dev); > pm_runtime_dont_use_autosuspend(&pdev->dev); > -- > 2.39.2 >
On Tue, 12 Sep 2023, Matt Roper <matthew.d.roper@intel.com> wrote: > On Tue, Sep 12, 2023 at 03:05:37PM +0300, Jani Nikula wrote: >> Don't hide display probe in device info code. >> >> Cc: Matt Roper <matthew.d.roper@intel.com> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/i915_driver.c | 2 ++ >> drivers/gpu/drm/i915/intel_device_info.c | 2 -- >> drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ >> 3 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c >> index f8dbee7a5af7..b5e1f72cc3ce 100644 >> --- a/drivers/gpu/drm/i915/i915_driver.c >> +++ b/drivers/gpu/drm/i915/i915_driver.c >> @@ -735,6 +735,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) >> /* Set up device info and initial runtime info. */ >> intel_device_info_driver_create(i915, pdev->device, match_info); >> >> + intel_display_device_probe(i915); >> + >> return i915; >> } >> >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c >> index b9b8b22540cb..d2ed0f057cb2 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.c >> +++ b/drivers/gpu/drm/i915/intel_device_info.c >> @@ -418,8 +418,6 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, >> runtime = RUNTIME_INFO(i915); >> memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); >> >> - intel_display_device_probe(i915); >> - >> runtime->device_id = device_id; >> } >> >> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >> index da0b269606c5..7de6477803f8 100644 >> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c >> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >> @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) >> /* Set up device info and initial runtime info. */ >> intel_device_info_driver_create(i915, pdev->device, &mock_info); >> >> + intel_display_device_probe(i915); >> + > > I realize this call was already happening with the old code, but do we > really want to be probing the (real) display hardware while setting up > mock (fake hardware) selftests? I don't think any of the mock tests > should be doing anything display-related, and if they did we'd probably > want to add some kind of mock_display_device_probe() instead or using > the real hardware? Thought about it, probably not, but decided it should be a separate patch. BR, Jani. > > > Matt > >> dev_pm_domain_set(&pdev->dev, &pm_domain); >> pm_runtime_enable(&pdev->dev); >> pm_runtime_dont_use_autosuspend(&pdev->dev); >> -- >> 2.39.2 >>
On Wed, 13 Sep 2023, Jani Nikula <jani.nikula@intel.com> wrote: > On Tue, 12 Sep 2023, Matt Roper <matthew.d.roper@intel.com> wrote: >> On Tue, Sep 12, 2023 at 03:05:37PM +0300, Jani Nikula wrote: >>> Don't hide display probe in device info code. >>> >>> Cc: Matt Roper <matthew.d.roper@intel.com> >>> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_driver.c | 2 ++ >>> drivers/gpu/drm/i915/intel_device_info.c | 2 -- >>> drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ >>> 3 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c >>> index f8dbee7a5af7..b5e1f72cc3ce 100644 >>> --- a/drivers/gpu/drm/i915/i915_driver.c >>> +++ b/drivers/gpu/drm/i915/i915_driver.c >>> @@ -735,6 +735,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) >>> /* Set up device info and initial runtime info. */ >>> intel_device_info_driver_create(i915, pdev->device, match_info); >>> >>> + intel_display_device_probe(i915); >>> + >>> return i915; >>> } >>> >>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c >>> index b9b8b22540cb..d2ed0f057cb2 100644 >>> --- a/drivers/gpu/drm/i915/intel_device_info.c >>> +++ b/drivers/gpu/drm/i915/intel_device_info.c >>> @@ -418,8 +418,6 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, >>> runtime = RUNTIME_INFO(i915); >>> memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); >>> >>> - intel_display_device_probe(i915); >>> - >>> runtime->device_id = device_id; >>> } >>> >>> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>> index da0b269606c5..7de6477803f8 100644 >>> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c >>> @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) >>> /* Set up device info and initial runtime info. */ >>> intel_device_info_driver_create(i915, pdev->device, &mock_info); >>> >>> + intel_display_device_probe(i915); >>> + >> >> I realize this call was already happening with the old code, but do we >> really want to be probing the (real) display hardware while setting up >> mock (fake hardware) selftests? I don't think any of the mock tests >> should be doing anything display-related, and if they did we'd probably >> want to add some kind of mock_display_device_probe() instead or using >> the real hardware? > > Thought about it, probably not, but decided it should be a separate > patch. Interestingly, the mock code does not set pdev->device, i.e. the device ID remains 0, and the old style display code would find no display. However, the GMD ID check goes ahead and probes display. So the mock device will have display on recognized GMD ID machines. Should fix that. BR, Jani. > > BR, > Jani. > >> >> >> Matt >> >>> dev_pm_domain_set(&pdev->dev, &pm_domain); >>> pm_runtime_enable(&pdev->dev); >>> pm_runtime_dont_use_autosuspend(&pdev->dev); >>> -- >>> 2.39.2 >>>
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index f8dbee7a5af7..b5e1f72cc3ce 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -735,6 +735,8 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) /* Set up device info and initial runtime info. */ intel_device_info_driver_create(i915, pdev->device, match_info); + intel_display_device_probe(i915); + return i915; } diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index b9b8b22540cb..d2ed0f057cb2 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -418,8 +418,6 @@ void intel_device_info_driver_create(struct drm_i915_private *i915, runtime = RUNTIME_INFO(i915); memcpy(runtime, &INTEL_INFO(i915)->__runtime, sizeof(*runtime)); - intel_display_device_probe(i915); - runtime->device_id = device_id; } diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index da0b269606c5..7de6477803f8 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) /* Set up device info and initial runtime info. */ intel_device_info_driver_create(i915, pdev->device, &mock_info); + intel_display_device_probe(i915); + dev_pm_domain_set(&pdev->dev, &pm_domain); pm_runtime_enable(&pdev->dev); pm_runtime_dont_use_autosuspend(&pdev->dev);
Don't hide display probe in device info code. Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_driver.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.c | 2 -- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-)