Re: [PATCH 1/6] drm/i915: unconditionally copy mode into crtc at boot time

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 12 Dec 2013 14:44:33 -0800
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Thu, 12 Dec 2013 23:39:39 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Thu, Dec 12, 2013 at 01:29:39PM -0800, Jesse Barnes wrote:
> > > On Thu, 12 Dec 2013 22:21:29 +0100
> > > Daniel Vetter <daniel@ffwll.ch> wrote:
> > > 
> > > > On Thu, Dec 12, 2013 at 12:41:52PM -0800, Jesse Barnes wrote:
> > > > > The BIOS code will need this to properly inherit the mode.
> > > > > 
> > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > > > index af3717a..1ae3d44 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -11175,7 +11175,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> > > > >  	 */
> > > > >  	list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> > > > >  			    base.head) {
> > > > > -		if (crtc->active && i915_fastboot) {
> > > > > +		if (crtc->active) {
> > > > 
> > > > That's just enabling half of the fastboot hack, so nacked.
> > > 
> > > This part isn't the hack, but is required for fastboot.  Without this,
> > > we'll end up with a bogus mode when we try to inherit from the BIOS.
> > > So if you want to nack this I'll have to put the other BIOS bits under
> > > fastboot as well.
> > 
> > crtc->config.pipe_src_w/h not good enough? I've thought that's what you've
> > used in the last iteration, hence my suprise why we suddenly need to
> > resurrect this hack here. All the information this hack exposes to
> > crtc->mode is available in the pipe config, so I really don't think you
> > neeed it.
> > 
> > Count me confused for now, please explain.
> 
> Yes, we read out all the data into the pipe config.  But if we don't
> put that data into the CRTC proper, any subsequent code that uses the
> CRTC mode will fail and think there's nothing there (resulting in fail
> at fbcon DPMS time for example).

The actual warning w/o this change is:

[   17.088591] [drm:intel_pipe_config_compare] *ERROR* mismatch in pipe_src_w (expected 0, found 4096)
[   17.088592] ------------[ cut here ]------------
[   17.088619] WARNING: CPU: 1 PID: 534 at drivers/gpu/drm/i915/intel_display.c:9588 check_crtc_state+0x6bf/0xc90 [i915]()
[   17.088619] pipe state doesn't match!
...
[   17.088710]  [<ffffffffa02f2caf>] check_crtc_state+0x6bf/0xc90 [i915]
[   17.088729]  [<ffffffffa03007ab>] intel_modeset_check_state+0x2bb/0x7b0 [i915]
[   17.088745]  [<ffffffffa0300d35>] intel_set_mode+0x25/0x30 [i915]
[   17.088762]  [<ffffffffa03015db>] intel_crtc_set_config+0x7ab/0x9a0 [i915]
[   17.088777]  [<ffffffffa0133a4d>] drm_mode_set_config_internal+0x5d/0xe0 [drm]
[   17.088783]  [<ffffffffa0184ea1>] drm_fb_helper_set_par+0x71/0xf0 [drm_kms_helper]
[   17.088787]  [<ffffffff8135b301>] fb_set_var+0x191/0x430

So the first mode set from the fb layer is half baked, because the core
DRM structures didn't have the right mode to pass down, and so we fall
over.

I can fix this either by always copying the mode info into the core
structs, or by putting the call to fbdev_init_bios under i915_fastboot
as well.

-- 
Jesse Barnes, Intel Open Source Technology Center