From patchwork Thu Jul 26 18:49:17 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1245191 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 80911DFFCE for ; Thu, 26 Jul 2012 20:41:00 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2F1A79EB08 for ; Thu, 26 Jul 2012 13:41:00 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 24F359EC4F for ; Thu, 26 Jul 2012 12:57:18 -0700 (PDT) Received: by mail-wi0-f171.google.com with SMTP id hq4so5480612wib.12 for ; Thu, 26 Jul 2012 12:57:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=8qlAIwoxzaH5QzSqxr3M0OigFQSnIb6l5wOxWakr32M=; b=QVD8NVMBhroNTHt1ZQGAw8/B9nIZHEF6UHMETIbYPongCQwTYK0NC3vmxPR5JiwDxx +z/TBY0n+9sETsWefeeTnQI7TjfhwYUbddBT0Mc3ybvtg5jqPSk0r/aiSyxJapTqrTWp BE5SHe2KO05eqogUU+m4Pme+2+CERzIQ4NADs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=8qlAIwoxzaH5QzSqxr3M0OigFQSnIb6l5wOxWakr32M=; b=HcV+VNNj2fbThLRBIoqSgJCnJmwR1hhHwv9zKuETAYFb8zSorL/vRLMkxtMYLNb5xh WZgfw8E13cYj78YAaj9zrocKibDTc0/uC5L1U1BF55qMK/UvcWWGWFOM6dMa1qTHeiDT 5/0ZGzHBwXFuSyXQqwUWrlKY66gyAOwd3KkYS19KA9LhuKrSwzZLyf30v0o7YkdhZjuY cBKmvkK9B3G+kNfAw0neihiT+tDoGaE00n9xp/2CZo9nm94sZmMPUtyr1+yZYYIvUjWc LrUv80ptITq/hMXRDTc7o+DiWEHJSMST1Vu95dc2JHXg+k8zTHlJzPhPjgJyAyvqpuXR ZxIA== Received: by 10.180.83.234 with SMTP id t10mr583746wiy.0.1343332638902; Thu, 26 Jul 2012 12:57:18 -0700 (PDT) Received: from wespe.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id fr4sm391403wib.8.2012.07.26.12.57.17 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 26 Jul 2012 12:57:18 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Thu, 26 Jul 2012 20:49:17 +0200 Message-Id: <1343328581-2324-53-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1343328581-2324-1-git-send-email-daniel.vetter@ffwll.ch> References: <1343328581-2324-1-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQkqdPZ58fV7JKfKVJT962pOEfK3b1Rx4scCKrnc+JAkBenCkH/ZdSH1TBG9XNCiaKqYsG24 Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 52/76] drm/i915: push crtc->fb update into pipe_set_base X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Passing in the old fb, having overwritten the current fb, leads to some neatly convoluted code. It's much simpler if we defer the crtc->fb update to the place that updates the hw, in pipe_set_base. This way we also don't need to restore anything in case something fails - we only update crtc->fb once things have succeeded. The real reason for this change is that now we keep the old fb assigned to crtc->fb, which allows us to finally move the crtc disable case into the common low-level set_mode function in the next patch. Also don't clobber crtc->x and crtc->y, we neatly pass these down the callchain already. Unfortunately we can't do the same with crtc->mode, because that one is being used in the mode_set callbacks. Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 84 ++++++++++++++-------------------- 1 file changed, 34 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 23c0d0f..a691c76 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2200,16 +2200,17 @@ intel_finish_fb(struct drm_framebuffer *old_fb) static int intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *old_fb) + struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_master_private *master_priv; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_framebuffer *old_fb; int ret; /* no fb bound */ - if (!crtc->fb) { + if (!fb) { DRM_ERROR("No FB bound\n"); return 0; } @@ -2223,7 +2224,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, mutex_lock(&dev->struct_mutex); ret = intel_pin_and_fence_fb_obj(dev, - to_intel_framebuffer(crtc->fb)->obj, + to_intel_framebuffer(fb)->obj, NULL); if (ret != 0) { mutex_unlock(&dev->struct_mutex); @@ -2231,17 +2232,20 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, return ret; } - if (old_fb) - intel_finish_fb(old_fb); + if (crtc->fb) + intel_finish_fb(crtc->fb); - ret = dev_priv->display.update_plane(crtc, crtc->fb, x, y); + ret = dev_priv->display.update_plane(crtc, fb, x, y); if (ret) { - intel_unpin_fb_obj(to_intel_framebuffer(crtc->fb)->obj); + intel_unpin_fb_obj(to_intel_framebuffer(fb)->obj); mutex_unlock(&dev->struct_mutex); DRM_ERROR("failed to update base address\n"); return ret; } + old_fb = crtc->fb; + crtc->fb = fb; + if (old_fb) { intel_wait_for_vblank(dev, intel_crtc->pipe); intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj); @@ -3776,6 +3780,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) * true if they don't match). */ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc, + struct drm_framebuffer *fb, unsigned int *pipe_bpp, struct drm_display_mode *mode) { @@ -3856,7 +3861,7 @@ static bool intel_choose_pipe_bpp_dither(struct drm_crtc *crtc, * also stays within the max display bpc discovered above. */ - switch (crtc->fb->depth) { + switch (fb->depth) { case 8: bpc = 8; /* since we go through a colormap */ break; @@ -4275,7 +4280,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, int x, int y, - struct drm_framebuffer *old_fb) + struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -4465,7 +4470,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc, I915_WRITE(DSPCNTR(plane), dspcntr); POSTING_READ(DSPCNTR(plane)); - ret = intel_pipe_set_base(crtc, x, y, old_fb); + ret = intel_pipe_set_base(crtc, x, y, fb); intel_update_watermarks(dev); @@ -4623,7 +4628,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, int x, int y, - struct drm_framebuffer *old_fb) + struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -4743,7 +4748,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, /* determine panel color depth */ temp = I915_READ(PIPECONF(pipe)); temp &= ~PIPE_BPC_MASK; - dither = intel_choose_pipe_bpp_dither(crtc, &pipe_bpp, mode); + dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode); switch (pipe_bpp) { case 18: temp |= PIPE_6BPC; @@ -5012,7 +5017,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, I915_WRITE(DSPCNTR(plane), dspcntr); POSTING_READ(DSPCNTR(plane)); - ret = intel_pipe_set_base(crtc, x, y, old_fb); + ret = intel_pipe_set_base(crtc, x, y, fb); intel_update_watermarks(dev); @@ -5025,7 +5030,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode, int x, int y, - struct drm_framebuffer *old_fb) + struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; @@ -5036,7 +5041,7 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, drm_vblank_pre_modeset(dev, pipe); ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode, - x, y, old_fb); + x, y, fb); drm_vblank_post_modeset(dev, pipe); return ret; @@ -5646,7 +5651,7 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, struct drm_encoder *encoder = &intel_encoder->base; struct drm_crtc *crtc = NULL; struct drm_device *dev = encoder->dev; - struct drm_framebuffer *old_fb; + struct drm_framebuffer *fb; int i = -1; DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", @@ -5707,8 +5712,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, if (!mode) mode = &load_detect_mode; - old_fb = crtc->fb; - /* We need a framebuffer large enough to accommodate all accesses * that the plane may generate whilst we perform load detection. * We can not rely on the fbcon either being present (we get called @@ -5716,19 +5719,19 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, * not even exist) or that it is large enough to satisfy the * requested mode. */ - crtc->fb = mode_fits_in_fbdev(dev, mode); - if (crtc->fb == NULL) { + fb = mode_fits_in_fbdev(dev, mode); + if (fb == NULL) { DRM_DEBUG_KMS("creating tmp fb for load-detection\n"); - crtc->fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); - old->release_fb = crtc->fb; + fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); + old->release_fb = fb; } else DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n"); - if (IS_ERR(crtc->fb)) { + if (IS_ERR(fb)) { DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n"); goto fail; } - if (!intel_set_mode(crtc, mode, 0, 0, old_fb)) { + if (!intel_set_mode(crtc, mode, 0, 0, fb)) { DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); if (old->release_fb) old->release_fb->funcs->destroy(old->release_fb); @@ -5742,7 +5745,6 @@ bool intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, fail: connector->encoder = NULL; encoder->crtc = NULL; - crtc->fb = old_fb; return false; } @@ -6593,13 +6595,12 @@ static void intel_modeset_commit_output_state(struct drm_device *dev) bool intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *old_fb) + int x, int y, struct drm_framebuffer *fb) { struct drm_device *dev = crtc->dev; drm_i915_private_t *dev_priv = dev->dev_private; struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode; struct drm_encoder_helper_funcs *encoder_funcs; - int saved_x, saved_y; struct drm_encoder *encoder; bool ret = true; @@ -6613,15 +6614,11 @@ bool intel_set_mode(struct drm_crtc *crtc, saved_hwmode = crtc->hwmode; saved_mode = crtc->mode; - saved_x = crtc->x; - saved_y = crtc->y; /* Update crtc values up front so the driver can rely on them for mode * setting. */ crtc->mode = *mode; - crtc->x = x; - crtc->y = y; /* Pass our mode to the connectors and the CRTC to give them a chance to * adjust it according to limitations or connector properties, and also @@ -6652,7 +6649,7 @@ bool intel_set_mode(struct drm_crtc *crtc, /* Set up the DPLL and any encoders state that needs to adjust or depend * on the DPLL. */ - ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, old_fb); + ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, fb); if (!ret) goto done; @@ -6668,6 +6665,9 @@ bool intel_set_mode(struct drm_crtc *crtc, encoder_funcs->mode_set(encoder, mode, adjusted_mode); } + crtc->x = x; + crtc->y = y; + /* Now enable the clocks, plane, pipe, and connectors that we set up. */ dev_priv->display.crtc_enable(crtc); @@ -6686,8 +6686,6 @@ done: if (!ret) { crtc->hwmode = saved_hwmode; crtc->mode = saved_mode; - crtc->x = saved_x; - crtc->y = saved_y; } return ret; @@ -6916,7 +6914,6 @@ next_encoder: static int intel_crtc_set_config(struct drm_mode_set *set) { struct drm_device *dev; - struct drm_framebuffer *old_fb = NULL; struct drm_mode_set save_set; struct intel_set_config *config; int ret; @@ -6979,13 +6976,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set) DRM_DEBUG_KMS("attempting to set mode from" " userspace\n"); drm_mode_debug_printmodeline(set->mode); - old_fb = set->crtc->fb; - set->crtc->fb = set->fb; if (!intel_set_mode(set->crtc, set->mode, - set->x, set->y, old_fb)) { + set->x, set->y, set->fb)) { DRM_ERROR("failed to set mode on [CRTC:%d]\n", set->crtc->base.id); - set->crtc->fb = old_fb; ret = -EINVAL; goto fail; } @@ -6998,18 +6992,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set) } drm_helper_disable_unused_functions(dev); } else if (config->fb_changed) { - set->crtc->x = set->x; - set->crtc->y = set->y; - - old_fb = set->crtc->fb; - if (set->crtc->fb != set->fb) - set->crtc->fb = set->fb; ret = intel_pipe_set_base(set->crtc, - set->x, set->y, old_fb); - if (ret != 0) { - set->crtc->fb = old_fb; - goto fail; - } + set->x, set->y, set->fb); } intel_set_config_free(config);