From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/6] drm/i915: unconditionally copy mode into crtc at boot time Date: Tue, 17 Dec 2013 09:00:28 +0100 Message-ID: <20131217080028.GJ9804@phenom.ffwll.local> References: <1386880917-2951-1-git-send-email-jbarnes@virtuousgeek.org> <20131212212129.GR9804@phenom.ffwll.local> <20131212132939.3dc7993f@jbarnes-desktop> <20131212223938.GS9804@phenom.ffwll.local> <20131212144433.600fb66b@jbarnes-desktop> <20131216163540.249d7006@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f169.google.com (mail-ea0-f169.google.com [209.85.215.169]) by gabe.freedesktop.org (Postfix) with ESMTP id AD6C0FC006 for ; Mon, 16 Dec 2013 23:59:37 -0800 (PST) Received: by mail-ea0-f169.google.com with SMTP id l9so2312098eaj.0 for ; Mon, 16 Dec 2013 23:59:36 -0800 (PST) Content-Disposition: inline In-Reply-To: <20131216163540.249d7006@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Dec 16, 2013 at 04:35:40PM -0800, Jesse Barnes wrote: > On Thu, 12 Dec 2013 14:44:33 -0800 > Jesse Barnes wrote: > > > On Thu, 12 Dec 2013 23:39:39 +0100 > > Daniel Vetter 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 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 > > > > > > --- > > > > > > 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] [] check_crtc_state+0x6bf/0xc90 [i915] > [ 17.088729] [] intel_modeset_check_state+0x2bb/0x7b0 [i915] > [ 17.088745] [] intel_set_mode+0x25/0x30 [i915] > [ 17.088762] [] intel_crtc_set_config+0x7ab/0x9a0 [i915] > [ 17.088777] [] drm_mode_set_config_internal+0x5d/0xe0 [drm] > [ 17.088783] [] drm_fb_helper_set_par+0x71/0xf0 [drm_kms_helper] > [ 17.088787] [] 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. Hm, I think I need to take a closer look here again since I really thought it'd would Just Work. Hiding more behind the fastboot hack isn't really what I want either, especially since we want to move from checking the input mode to checking the pipe config for fastbooting. So having a single piece which relies on that reconstructed mode would be rather ugly. Can you please attach the full debug log for this? Also, could this just be a side effect of the more fancy ->initial_config logic, i.e. what happens if we disable that one? Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch