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

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Dec 12, 2013 at 02:44:33PM -0800, Jesse Barnes 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).

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