From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ander Conselvan de Oliveira Subject: Re: [PATCH v2] drm/i915: Initialize primary plane src/dst coords when reading hw state Date: Tue, 20 Jan 2015 11:32:54 +0200 Message-ID: <54BE20C6.3040604@intel.com> References: <1421077012-25369-1-git-send-email-ander.conselvan.de.oliveira@intel.com> <1421675507-27529-1-git-send-email-ander.conselvan.de.oliveira@intel.com> <20150120092211.GH10113@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format="flowed" Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <20150120092211.GH10113@phenom.ffwll.local> Sender: stable-owner@vger.kernel.org To: Daniel Vetter Cc: =?windows-1252?Q?Ville_Syrj=E4l=E4?= , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On 01/20/2015 11:22 AM, Daniel Vetter wrote: > On Mon, Jan 19, 2015 at 03:51:47PM +0200, Ander Conselvan de Oliveira wrote: >> Otherwise setting the rotation property will cause the primary plane to >> be disabled, caused by having a 0x0 initial value. >> >> v2: Rebase on top of the move to plane helpers. >> >> Cc: stable@vger.kernel.org >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87662 >> Signed-off-by: Ander Conselvan de Oliveira > > Testcase: igt/ksm_plane/*-suspend-* > >> --- >> drivers/gpu/drm/i915/intel_display.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 91d8ada..ed70ca7 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -13327,6 +13327,23 @@ static bool primary_get_hw_state(struct intel_crtc *crtc) >> return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE; >> } >> >> +static void primary_update_size(struct intel_crtc *crtc) >> +{ >> + struct drm_plane_state *state = crtc->base.primary->state; >> + >> + if (!crtc->primary_enabled) >> + return; >> + >> + state->crtc_x = 0; >> + state->crtc_y = 0; >> + state->crtc_w = crtc->config.pipe_src_w; >> + state->crtc_h = crtc->config.pipe_src_h; >> + state->src_x = 0; >> + state->src_y = 0; >> + state->src_w = crtc->config.pipe_src_w << 16; >> + state->src_h = crtc->config.pipe_src_h << 16; >> +} >> + >> static void intel_modeset_readout_hw_state(struct drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> @@ -13337,6 +13354,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) >> int i; >> >> for_each_intel_crtc(dev, crtc) { >> + > > Spurious hunk. > >> memset(&crtc->config, 0, sizeof(crtc->config)); >> >> crtc->config.quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE; >> @@ -13346,6 +13364,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) >> >> crtc->base.enabled = crtc->active; >> crtc->primary_enabled = primary_get_hw_state(crtc); >> + primary_update_size(crtc); > > I think Ville raised a really good point about the fragility of this and > restoring plane state correctly. I think conceptually it makes more sense > to restore the primary plane state together with the fb in the loop at the > end of intel_modeset_init. Would that still work, or is that too late for > when we change pipe state when sanititizing crtcs? That should work. Actually, Chris sent a patch that did that some time ago, and Ville commented that "[he was] thinking that for now these would be better placed in intel_modeset_readout_hw_state() where we read out the primary plane enabled state as well". [1] So just to make it complete clear, does the problem of restoring plane state correctly supersedes Ville's previous comment? [1] http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/46466 Ander > -Daniel > >> >> DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n", >> crtc->base.base.id, >> -- >> 1.9.1 >> > --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.