From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Do not clobber config status after a forced restore of hw state Date: Mon, 18 Nov 2013 17:28:33 +0100 Message-ID: <20131118162833.GE8203@phenom.ffwll.local> References: <1384785909-2869-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f171.google.com (mail-ea0-f171.google.com [209.85.215.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 92283FAD60 for ; Mon, 18 Nov 2013 08:27:57 -0800 (PST) Received: by mail-ea0-f171.google.com with SMTP id h10so2576370eak.30 for ; Mon, 18 Nov 2013 08:27:56 -0800 (PST) Content-Disposition: inline In-Reply-To: <1384785909-2869-1-git-send-email-chris@chris-wilson.co.uk> 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: Chris Wilson Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Nov 18, 2013 at 02:45:09PM +0000, Chris Wilson wrote: > We call intel_modeset_setup_hw_state() along two paths, driver > load/resume and after a lid event notification. During initialisation of > the driver, it is imperative that we reset the config state. This > correctly sets up the initial connector statuses and prepares the > hardware for a thorough probing. However, during a lid event, we only > want to undo the damage caused by the bios by resetting our last known > mode. In this cirumstance, we do not want to clobber our desired state. > > In order to try and keep sanity between the config state and our own > tracking, do the drm_mode_config_reset() first along the load/resume > paths before reading out the hw state and apply any definite known > corrections. > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_drv.c | 1 + > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index c2e00ed23195..66191875d632 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -648,6 +648,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings) > intel_modeset_init_hw(dev); > > drm_modeset_lock_all(dev); > + drm_mode_config_reset(dev); > intel_modeset_setup_hw_state(dev, true); > drm_modeset_unlock_all(dev); > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 271d9e526a72..d97c90586672 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11117,6 +11117,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > connector->base.dpms = DRM_MODE_DPMS_ON; > connector->encoder->connectors_active = true; > connector->base.encoder = &connector->encoder->base; > + connector->base.status = connector_status_connected; As discussed on irc I don't think we should force the connector state to anything here: Imo connector->status should reflect what we believe to be the true output connection state, whereas connector->encoder reflects whether this connector is wired up to a pipe. And since we no longer reject modeset on disconnected connectors and never nuked the pipe if the connector gets disconnected there's no reason for that - such policy is userspace's job. Note though that the multi-connector encoders are a bit botched still and select the connector according to the hotplug state (or some other semi-random piece of state) instead of obeying userspace's wish. With some resulting hilarity. Oh and: This regression has been introduced in commit 2e9388923e83bc4e2726f170a984621f1d582e77 Author: Daniel Vetter Date: Thu Oct 11 20:08:24 2012 +0200 drm/i915/crt: explicitly set up HOTPLUG_BITS on resume so needs Cc: stable@vger.kernel.org (v3.8 and later) Cc: Paulo Zanoni Cheers, Daniel > } else { > connector->base.dpms = DRM_MODE_DPMS_OFF; > connector->base.encoder = NULL; > @@ -11203,8 +11204,6 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, > } > > intel_modeset_check_state(dev); > - > - drm_mode_config_reset(dev); > } > > void intel_modeset_gem_init(struct drm_device *dev) > @@ -11213,6 +11212,7 @@ void intel_modeset_gem_init(struct drm_device *dev) > > intel_setup_overlay(dev); > > + drm_mode_config_reset(dev); > intel_modeset_setup_hw_state(dev, false); > } > > -- > 1.8.4.3 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch