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: Fri, 13 Dec 2013 00:04:44 +0100 Message-ID: <20131212230444.GU9804@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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f44.google.com (mail-ee0-f44.google.com [74.125.83.44]) by gabe.freedesktop.org (Postfix) with ESMTP id 7F860FA4FA for ; Thu, 12 Dec 2013 15:03:53 -0800 (PST) Received: by mail-ee0-f44.google.com with SMTP id b57so539655eek.3 for ; Thu, 12 Dec 2013 15:03:52 -0800 (PST) Content-Disposition: inline In-Reply-To: <20131212144433.600fb66b@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Dec 12, 2013 at 02:44:33PM -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). If fbcon dpms is fumbling around in crtc->mode that's a bug (or at least a pretty blantant layering violation). The one I know of is the crtc->hwmode fumbling the vblank code does. But iirc Ville has patches for that somewhere, and it's not really the worse offense in drm_irq.c. We obviously need crtc->fb for our code to be happy (since we haven't split off the primary planes yet). But anything beyond that shouldn't be required. So I'm still confused ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch