Re: [PATCH 3/6] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon v7

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Sat, 14 Dec 2013 12:36:30 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Sat, Dec 14, 2013 at 12:13:45PM +0100, Daniel Vetter wrote:
> > On Thu, Dec 12, 2013 at 12:41:54PM -0800, Jesse Barnes wrote:
> > > +	ifbdev->helper.funcs->initial_config = intel_fb_initial_config;
> > 
> > This here is a bit surprising - my model of operation here presumed that
> > if we correctly assign the crtc->fb and the ifbdev->fb pointers we could
> > fully rely on the fastboot setcrtc logic to eschew the modeset.
> > 
> > Being the ever-vary of special-purpose logic I'd much prefer this implicit
> > approach - otherwise we have one more special case to care about in the
> > fastboot=y/n and CONFIG_FB=y/n matrix.
> > 
> > So have you tried to ditch this special initial_config functions
> > (obviously only looks good with fastboot=1) or what precise corner-case
> > does this fix?
> 
> Ok, I've dug out your old patch from almost a year ago which added the
> ->initial_config hook. I see the point now of copying exactly the bios
> config in the hope that we end up with something that has a higher chance
> of working.
> 
> But imo this is an issue separate from the "take over bios fb" feature
> here, so this should be
> - split into a separate patch
> - used even when we fail to take over the bios fb
> The later point will require some mode-from-pipe_config reconstruction to
> work outside of the fastboot=1 hack mode.
> 
> I really like the idea though.

Ok I split this up, made fb a ptr, fixed up the CONFIG_FB bits, and I
think we figured out the crtc timing stuff.  I think that's all the
feedback from the last round, so I'll re-post for some (hopefully)
final review.

There are some additional improvements that would be nice:
  - compute_mode_changes needs to get smarter in general and look at
    pfit state.  Eventually we'll probably need a platform specific
    callback for this that tells us whether a pipe shutdown is needed
    for a given global configuration change.
  - pfit disable should be split out into a separate callback from our
    mode_set function (which also needs to get smarter after the
    compute_mode_changes improvements)
  - need to detect audio and infoframe configs and cross-check, though
    I don't think these affect fastboot at all since we ought to be
    able to leave them alone

All of the above aren't strictly necessary though, they're just
improvements on current code, some of which will overlap with the
atomic mode set work.  So we may be able to flip the i915.fastboot=1
default switch once these bits land after some additional HSW and BDW
testing...

Thanks,
Jesse