On Thu, 2018-08-09 at 11:16 +0300, Jani Nikula wrote:
> On Wed, 08 Aug 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.
>
> Okay, this gets confusing. There are other paths in
> intel_device_info_runtime_init() that set num_pipes = 0 and depend on
> PCH having been detected. :(
>
> >
> > 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 9dce55182c3a..7952f5877402 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -917,6 +917,11 @@ static int i915_driver_init_early(struct
> > drm_i915_private *dev_priv,
> > if (ret < 0)
> > goto err_workqueues;
> >
> > + if (i915_modparams.disable_display) {
> > + DRM_INFO("Display disabled (module parameter)\n");
> > + device_info->num_pipes = 0;
> > + }
> > +
>
> Please look at the function as a whole, and note that this feels like
> a
> random thing to add in the middle. Needs to be stowed away somewhere
> deeper.
I can move it to right after:
/* Setup the write-once "constant" device info */
device_info = mkwrite_device_info(dev_priv);
memcpy(device_info, match_info, sizeof(*device_info));
device_info->device_id = dev_priv->drm.pdev->device;
>
> Overall, I think we need to be more accurate about the relationship
> of
> num_pipes = 0 and PCH_NOP.
The path in intel_device_info_runtime_init() that now sets num_pipes =
0 is when the display(I guess it is the whole GPU) is fused off so user
can't use it at all.
The other path changing num_pipes is one for IVB there is disables the
last pipe.
I guess with this changes we have a good relationship between num_pipes
and PCH_NOP or do you have another suggestion.
>
>
> BR,
> Jani.
>
> > /* This must be called before any calls to HAS_PCH_* */
> > intel_detect_pch(dev_priv);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > b/drivers/gpu/drm/i915/intel_device_info.c
> > index 0ef0c6448d53..67102b481c8f 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -776,12 +776,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);
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
