From patchwork Thu Sep 26 22:51:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lyude Paul X-Patchwork-Id: 11163433 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7025317D4 for ; Thu, 26 Sep 2019 22:52:24 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 580CF20835 for ; Thu, 26 Sep 2019 22:52:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 580CF20835 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 0FF626EE2C; Thu, 26 Sep 2019 22:52:21 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 319D96EE2B; Thu, 26 Sep 2019 22:52:20 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BC6FF796ED; Thu, 26 Sep 2019 22:52:19 +0000 (UTC) Received: from malachite.bss.redhat.com (dhcp-10-20-1-34.bss.redhat.com [10.20.1.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id C54DA600C1; Thu, 26 Sep 2019 22:52:18 +0000 (UTC) From: Lyude Paul To: amd-gfx@lists.freedesktop.org Subject: [PATCH 6/6] drm/encoder: WARN() when adding/removing encoders after device registration Date: Thu, 26 Sep 2019 18:51:08 -0400 Message-Id: <20190926225122.31455-7-lyude@redhat.com> In-Reply-To: <20190926225122.31455-1-lyude@redhat.com> References: <20190926225122.31455-1-lyude@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Thu, 26 Sep 2019 22:52:19 +0000 (UTC) X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , linux-kernel@vger.kernel.org, Maxime Ripard , dri-devel@lists.freedesktop.org, Sean Paul Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Turns out that we don't actually check this anywhere, and additionally actually forget to even mention this in our documentation. Since we've had one driver making this mistake already, let's clarify this by mentioning this limitation in the kernel docs. Additionally, for drivers not using the legacy ->load/->unload() hooks (which make it impossible to create all encoders before registration): print a big warning when drm_encoder_init() is called after device registration, or when drm_encoder_cleanup() is called before device unregistration. Signed-off-by: Lyude Paul Cc: Ville Syrjälä --- drivers/gpu/drm/drm_encoder.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 80d88a55302e..5c5e40aafa4e 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -99,9 +99,12 @@ void drm_encoder_unregister_all(struct drm_device *dev) * @encoder_type: user visible type of the encoder * @name: printf style format string for the encoder name, or NULL for default name * - * Initialises a preallocated encoder. Encoder should be subclassed as part of - * driver encoder objects. At driver unload time drm_encoder_cleanup() should be - * called from the driver's &drm_encoder_funcs.destroy hook. + * Initialises a preallocated encoder. The encoder should be subclassed as + * part of driver encoder objects. This function may not be called after the + * device is registered with drm_dev_register(). + * + * At driver unload time drm_encoder_cleanup() must be called from the + * driver's &drm_encoder_funcs.destroy hook. * * Returns: * Zero on success, error code on failure. @@ -117,6 +120,14 @@ int drm_encoder_init(struct drm_device *dev, if (WARN_ON(dev->mode_config.num_encoder >= 32)) return -EINVAL; + /* + * Encoders should never be added after device registration, with the + * exception of drivers using the legacy load/unload callbacks where + * it's impossible to create encoders beforehand. Such drivers should + * convert to using drm_dev_register() and friends. + */ + WARN_ON(dev->registered && !dev->driver->load); + ret = drm_mode_object_add(dev, &encoder->base, DRM_MODE_OBJECT_ENCODER); if (ret) return ret; @@ -155,16 +166,22 @@ EXPORT_SYMBOL(drm_encoder_init); * drm_encoder_cleanup - cleans up an initialised encoder * @encoder: encoder to cleanup * - * Cleans up the encoder but doesn't free the object. + * Cleans up the encoder but doesn't free the object. This function may not be + * called until the respective &struct drm_device has been unregistered from + * userspace using drm_dev_unregister(). */ void drm_encoder_cleanup(struct drm_encoder *encoder) { struct drm_device *dev = encoder->dev; - /* Note that the encoder_list is considered to be static; should we - * remove the drm_encoder at runtime we would have to decrement all - * the indices on the drm_encoder after us in the encoder_list. + /* + * Encoders should never be removed after device registration, with + * the exception of drivers using the legacy load/unload callbacks + * where it's impossible to remove encoders until after + * deregistration. Such drivers should convert to using + * drm_dev_register() and friends. */ + WARN_ON(dev->registered && !dev->driver->unload); if (encoder->bridge) { struct drm_bridge *bridge = encoder->bridge;