Re: [PATCH 5/5] drm/i915: enable fastboot by default

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jun 10, 2014 at 11:42:37AM -0700, Jesse Barnes wrote:
> On Tue, 10 Jun 2014 11:01:06 -0700
> Stéphane Marchesin <stephane.marchesin@gmail.com> wrote:
> 
> > On Tue, Jun 10, 2014 at 10:31 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > On Tue, 10 Jun 2014 16:07:44 +0200
> > > Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > >> On Thu, Jun 05, 2014 at 11:24:31AM -0700, Jesse Barnes wrote:
> > >> > Let them eat mincemeat pie.
> > >> >
> > >> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > >> > ---
> > >> >  drivers/gpu/drm/i915/i915_params.c | 4 ++--
> > >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > >> > index d05a2af..081ab2f 100644
> > >> > --- a/drivers/gpu/drm/i915/i915_params.c
> > >> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > >> > @@ -41,7 +41,7 @@ struct i915_params i915 __read_mostly = {
> > >> >     .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT),
> > >> >     .disable_power_well = 1,
> > >> >     .enable_ips = 1,
> > >> > -   .fastboot = 0,
> > >> > +   .fastboot = 42,
> > >> >     .prefault_disable = 0,
> > >> >     .reset = true,
> > >> >     .invert_brightness = 0,
> > >> > @@ -132,7 +132,7 @@ MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)");
> > >> >
> > >> >  module_param_named(fastboot, i915.fastboot, bool, 0600);
> > >> >  MODULE_PARM_DESC(fastboot,
> > >> > -   "Try to skip unnecessary mode sets at boot time (default: false)");
> > >> > +   "Try to skip unnecessary mode sets at boot time (default: true)");
> > >>
> > >> Nah, that wasn't the intention of this option. It was meant as a hack to
> > >> experiment around with fastboot and get things going, but imo we need to
> > >> really do the full modeset and short-circuit if the state matches.
> > >>
> > >> And there's still a bunch of things we don't track like infoframes which
> > >> we either need to fix up (similar to the pfit fixup) or quirk to disallow
> > >> fastboot.
> > >
> > > Hm that contradicts our earlier discussions w/Damien when we decided
> > > the infoframes stuff were too esoteric to matter...

I'm pretty sure I've never claimed that infoframes are too esoteric. Like
Stéphane mentions they're really good at breaking shitty TVs and resulting
in black screens.

> > My 2 cents is that I've seen some really bad TVs which didn't work
> > because infoframes were missing (IIRC it relied on the VIC to detect
> > the video mode).
> 
> Yeah so we'd still leave them in place in this case, and apply them on
> the next mode set as well, but we wouldn't be explicitly cross checking
> for them, at least not yet.
> 
> It's a good thing to add, I just didn't think it was a blocker based on
> our last discussion about this.

Imo we should quirk hdmi and prevent fastboot exactly because infoframes
are such a pain. Relying on the BIOS for the just means we'll loose a lot
of testing coverage on random screens out there, which I very much want to
avoid.

Another piece of state we don't fix up atm is sound. Since my latest
rework this is tracked in the pipe config, so at least we'll catch that.

Also iirc Chris complained about the modeset state checker overhead caused
by the fastboot option since a normal flip done through setCrtc now hits
that unconditionally. If we'd push the fastboot logic into the overall
modeset path we could restrict modeset state checks to only when we need
them.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch