From patchwork Thu Jan 24 16:20:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 2033731 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id E84343FDBC for ; Thu, 24 Jan 2013 16:31:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D9EF5E6CAF for ; Thu, 24 Jan 2013 08:31:53 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wi0-f182.google.com (mail-wi0-f182.google.com [209.85.212.182]) by gabe.freedesktop.org (Postfix) with ESMTP id C3752E6C76 for ; Thu, 24 Jan 2013 08:21:04 -0800 (PST) Received: by mail-wi0-f182.google.com with SMTP id hn14so615749wib.3 for ; Thu, 24 Jan 2013 08:21:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=OQCAnki7UO5cpuV4ODKOsJuf1DIBz8ru3YkAxeLwazM=; b=JUKLvOLaKx2rAufonDTwArFZ7tLum5Hl3qsq7UwvytEHb0Uga9vOUGeP/N9myJkaYd 7HUBc27O6TStsyIo3vUzhGbMAW0nsCr/RZ2So7okbWK/EEwqoMiB9A32okLY6jHeFl+p /KdWl/Wye9e31FrEgQBhfyQ8StW3Ew01nB2OI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=OQCAnki7UO5cpuV4ODKOsJuf1DIBz8ru3YkAxeLwazM=; b=jCKnFbd0sYeHN/d55AAsdIq7v5x60BwDGgdDH6pPBNGE6k+Yqo4q4ZCFUWDm+TcFPn IYj/zRs6zbOCVqeHcRQnvauI20Nqe2JvFHqjV2U/KhjQmCYlLD/2zeTjlc2XlqyRJXS3 sH9LNQ5Hx7VUs/JhpysLE7zTCa98Uts3BqDyRAZw37SM7IRUyiL30DTxHzTgrdMHE1sG yGnoLTwNC6PuyctDi8z3Ao1DC3HRhA4HWa45tVsjFqavtI0oQgqs9NjlgKtXo9c+wG76 Uj8X6w/5pHV6CcUuYLmtSfhD7vchqEZeWDnFo8IfyfcnZpFbogSqyPneFWhH5lbo0Dl8 r09g== X-Received: by 10.194.19.10 with SMTP id a10mr4144504wje.45.1359044463888; Thu, 24 Jan 2013 08:21:03 -0800 (PST) Received: from biers.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id l5sm3273370wia.10.2013.01.24.08.21.03 (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 24 Jan 2013 08:21:03 -0800 (PST) From: Daniel Vetter To: DRI Development Subject: [PATCH 11/16] drm/fb-helper: fixup up set_config semantics Date: Thu, 24 Jan 2013 17:20:43 +0100 Message-Id: <1359044448-3861-12-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1359044448-3861-1-git-send-email-daniel.vetter@ffwll.ch> References: <1359044448-3861-1-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQnVctBZi8QvA4AzinoADefBnUbNl/afymG5Vc9TqrzJyQu3HbaXC35vPAUsxrTnKdDgj6GF Cc: Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org While doing the modeset rework for drm/i915 I've noticed that the fb helper is very liberal with the semantics of the ->set_config interface: - It doesn't bother clearing stale modes (e.g. when unpluggint a screen). - It unconditionally sets and the fb, even if no mode will be set on a given crtc. - The initial setup is a bit fun since we need to pick crtcs to decide the desired fb size, but also should set the modeset->fb pointer. Explain what's going on in the fixup code after the fb is allocated. The crtc helper didn't really care, but the new i915 modeset infrastructure did, so I've had to add a bunch of special-cases to catch this. Fix this all up and enforce the interface by converting the checks in drm/i915/intel_display.c to BUG_ONs. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/intel_display.c | 11 +++-------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index dbf0020..5c73a12 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -689,7 +689,6 @@ int drm_fb_helper_set_par(struct fb_info *info) struct drm_fb_helper *fb_helper = info->par; struct drm_device *dev = fb_helper->dev; struct fb_var_screeninfo *var = &info->var; - struct drm_crtc *crtc; int ret; int i; @@ -700,7 +699,6 @@ int drm_fb_helper_set_par(struct fb_info *info) drm_modeset_lock_all(dev); for (i = 0; i < fb_helper->crtc_count; i++) { - crtc = fb_helper->crtc_info[i].mode_set.crtc; ret = drm_mode_set_config_internal(&fb_helper->crtc_info[i].mode_set); if (ret) { drm_modeset_unlock_all(dev); @@ -841,9 +839,17 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, info = fb_helper->fbdev; - /* set the fb pointer */ + /* + * Set the fb pointer - usually drm_setup_crtcs does this for hotplug + * events, but at init time drm_setup_crtcs needs to be called before + * the fb is allocated (since we need to figure out the desired size of + * the fb before we can allocate it ...). Hence we need to fix things up + * here again. + */ for (i = 0; i < fb_helper->crtc_count; i++) - fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb; + if (fb_helper->crtc_info[i].mode_set.num_connectors) + fb_helper->crtc_info[i].mode_set.fb = fb_helper->fb; + if (new_fb) { info->var.pixclock = 0; @@ -1314,6 +1320,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper) for (i = 0; i < fb_helper->crtc_count; i++) { modeset = &fb_helper->crtc_info[i].mode_set; modeset->num_connectors = 0; + modeset->fb = NULL; } for (i = 0; i < fb_helper->connector_count; i++) { @@ -1330,9 +1337,21 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper) modeset->mode = drm_mode_duplicate(dev, fb_crtc->desired_mode); modeset->connectors[modeset->num_connectors++] = fb_helper->connector_info[i]->connector; + modeset->fb = fb_helper->fb; } } + /* Clear out any old modes if there are no more connected outputs. */ + for (i = 0; i < fb_helper->crtc_count; i++) { + modeset = &fb_helper->crtc_info[i].mode_set; + if (modeset->num_connectors == 0) { + BUG_ON(modeset->fb); + BUG_ON(modeset->num_connectors); + if (modeset->mode) + drm_mode_destroy(dev, modeset->mode); + modeset->mode = NULL; + } + } out: kfree(crtcs); kfree(modes); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 055b24a..7e3dd0f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7940,14 +7940,9 @@ static int intel_crtc_set_config(struct drm_mode_set *set) BUG_ON(!set->crtc); BUG_ON(!set->crtc->helper_private); - if (!set->mode) - set->fb = NULL; - - /* The fb helper likes to play gross jokes with ->mode_set_config. - * Unfortunately the crtc helper doesn't do much at all for this case, - * so we have to cope with this madness until the fb helper is fixed up. */ - if (set->fb && set->num_connectors == 0) - return 0; + /* Enforce sane interface api - has been abused by the fb helper. */ + BUG_ON(!set->mode && set->fb); + BUG_ON(set->fb && set->num_connectors == 0); if (set->fb) { DRM_DEBUG_KMS("[CRTC:%d] [FB:%d] #connectors=%d (x y) (%i %i)\n",