On Fri, 12 Oct 2018, José Roberto de Souza <jose.souza@xxxxxxxxx> wrote: > num_pipes is set to 0 if disable_display is set inside > intel_device_info_runtime_init() but when that happen PCH will > already be set in intel_detect_pch(). > > i915_driver_load() > i915_driver_init_early() > ... > intel_detect_pch() > ... > ... > i915_driver_init_hw() > intel_device_info_runtime_init() > > So now setting num_pipes = 0 earlier to avoid this problem. I'm growing this nagging feeling that this may be the wrong direction. It's just that disabling an existing display and not having display are two very different things. Even if our goal here is use the former to test the latter, I think it's more subtle than this. You already saw some of this in patch 15 of this series. You can't just pretend there's no display if you also want to gain benefits. I'm also not sure if we need to go as far as Chris is suggesting in [1], but assuming disable display means num_pipes = 0 from start prevents us from doing that later on, because we'll start making assumptions. Like you already do in this series. Again, there's a bunch of useful stuff in the series that could be merged with a different ordering. Including patches 3-5 that look superficially good but do need an in-depth review. The init sequences are hard, and we should add more asserts and comments on the ordering, because we tend to forget why things are the way they are. BR, Jani. [1] https://patchwork.freedesktop.org/series/51000/ > > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 +++++ > drivers/gpu/drm/i915/intel_device_info.c | 8 ++------ > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index baac35f698f9..e3efc3dd8a30 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1649,6 +1649,11 @@ i915_driver_create(struct pci_dev *pdev, const struct pci_device_id *ent) > memcpy(device_info, match_info, sizeof(*device_info)); > device_info->device_id = pdev->device; > > + if (i915_modparams.disable_display) { > + DRM_INFO("Display disabled (module parameter)\n"); > + device_info->num_pipes = 0; > + } > + > BUILD_BUG_ON(INTEL_MAX_PLATFORMS > > BITS_PER_TYPE(device_info->platform_mask)); > BUG_ON(device_info->gen > BITS_PER_TYPE(device_info->gen_mask)); > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index 03df4e33763d..69be3f211737 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -775,12 +775,8 @@ void intel_device_info_runtime_init(struct intel_device_info *info) > info->num_sprites[pipe] = 1; > } > > - if (i915_modparams.disable_display) { > - DRM_INFO("Display disabled (module parameter)\n"); > - info->num_pipes = 0; > - } else if (info->num_pipes > 0 && > - (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) && > - HAS_PCH_SPLIT(dev_priv)) { > + if (info->num_pipes > 0 && (IS_GEN7(dev_priv) || IS_GEN8(dev_priv)) && > + HAS_PCH_SPLIT(dev_priv)) { > u32 fuse_strap = I915_READ(FUSE_STRAP); > u32 sfuse_strap = I915_READ(SFUSE_STRAP); -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
