[Intel-gfx] [PATCH 4/3] drm/i915: Clarify logic for initial modeset

[Daniel Vetter]

On Wed, Jul 15, 2015 at 03:20:34PM +0200, Maarten Lankhorst wrote:
> Hey,
> 
> Op 15-07-15 om 14:15 schreef Daniel Vetter:
> > Currently we both set mode->private_flags to some value and also use
> > the pipe_config quirk. But since the pipe_config quirk isn't tied to
> > the lifetime of the mode object we need to check both.
> >
> > Simplify this by only using mode.private_flags and stop using the
> > INHERITED_MODE quirk. Also for clarity add an explicit #define for
> > that driver priavete mode flag.
> >
> > By using crtc_state->mode_changed we can also remove the recalc local
> > variable.
> Those 3 changes look good to me.
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> 
> Any objections against this followup patch?
> Or should I wait until I made intel_update_pipe_size atomic?
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 7eff33ff84f6..914679ceb200 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -578,9 +578,6 @@ static bool intel_fbdev_init_bios(struct drm_device *dev,
>  	struct intel_crtc *intel_crtc;
>  	unsigned int max_size = 0;
>  
> -	if (!i915.fastboot)
> -		return false;

I tried to digg out from history why we added that check, but never found
it. Original commit says that Jesse added this on his own, but doesn't
explain the reason for that. Reading through the code there doesn't seem
to be anything in there that needs other bits of fastboot (or ever needed
other bits of fastboot). The only effect I can see is that we'll take over
the bpp setting from the firmware, and previously that resulted in
modesets since we used the primary bpp to pick the pipe bpp. But that's
fixed now so no risk for spurious modesets when doing fbcon<->X vt
switches.

Can you please paste the above explanation into a commit message and send
it out with sob and all? I'll pick it up.

Cheers, Daniel
> -
>  	/* Find the largest fb */
>  	for_each_crtc(dev, crtc) {
>  		struct drm_i915_gem_object *obj =
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch