Received: by 2002:a05:6358:d09b:b0:dc:cd0c:909e with SMTP id jc27csp5662158rwb; Tue, 22 Nov 2022 03:16:18 -0800 (PST) X-Google-Smtp-Source: AA0mqf6G5yloWIiK0SOj8RVzJY5tCaiYUWYHq4WS0UPeZEbWwDgPPpLtBeNIplZyqw2Cukxt8itQ X-Received: by 2002:a17:906:b5a:b0:7ae:1874:3e09 with SMTP id v26-20020a1709060b5a00b007ae18743e09mr2451110ejg.534.1669115777869; Tue, 22 Nov 2022 03:16:17 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669115777; cv=none; d=google.com; s=arc-20160816; b=vfuyeqvQu6czhDbBGLzvfdS4Wq3TJKrFVCXmby4g1MF/Rgb/tFM0jG2JKJEj4ILcVG 9Ft60JQqVqEOoj8LsPYLbe3brOr6CRrpmAeKp74EuqR4mQTRu1cUyoaiqGSX6Xlti/OF PkW9ansRoVp8g64pVzW7nZohNZa5CiSO2SfVXv8gL7lfeOJKzgdfybzLF0/XWVcjFdZx dCDesNpuNJoewy/vTedTQfX9nsMy71DT1aPI8RwH1qQ52me5I1f33qYT05GqZwHQUEEv ox0N/eQxCROK3aEEI31dAVTib9BLrQ3/rOp4CEkhIEfqwWrbtPZvmsD5AUBrxZPv3zmf aimA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=FNZKQs7jhT+OIaLLceURPBnRl7Yy+jlg8eMePewKd+M=; b=uTDVRXE1nxag1BGJMr/f2lA6qLiYaZSsYpph3JgJR75r0Dw9+WM4y30ebBloiweB2B O2Qqkn0a4dsJwYwgvlZDxzd0oa/9NpZF7NQ2WUw83ZGPOXFVwH2iXtgZFcyz0F7WF50T PYGkEMxfPq5xVioRUi9S43eWMRH0S+X/7IzGwlMu/YTI+WVGpjpqhTTjqwux8WbM4ECV JZlg24fIXgsCYUhLH4P3M2rYRMLPtjQ8P+AICBnPXk9AGI7IFeZUfJFIRTvjP2VRHE1q N75aRoW0q83la+V5WVcgkTcDQ9PRznhqpoaVEjzkQWEtS5XmDy7NjsBO03l8e8/NpNP8 ymTg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=rfbivQDM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id cw7-20020a170906478700b007806964b2fcsi7465684ejc.447.2022.11.22.03.15.55; Tue, 22 Nov 2022 03:16:17 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=rfbivQDM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232080AbiKVLCg (ORCPT + 90 others); Tue, 22 Nov 2022 06:02:36 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53316 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232823AbiKVLCe (ORCPT ); Tue, 22 Nov 2022 06:02:34 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AC6D638BB for ; Tue, 22 Nov 2022 03:02:31 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 34F616160E for ; Tue, 22 Nov 2022 11:02:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 926D4C433C1 for ; Tue, 22 Nov 2022 11:02:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1669114950; bh=GBbyR90uPkkY9uQHcMLNsONxrGwJbliVqQEjjoEjnxw=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=rfbivQDMXLp5UZqwbuLeox3SpmxiIgL1eSVglkW/KT3oiaMBFSsiUICIxIo7oZWoc KR/iwzYU6dt23ahwdJ700ZP5EeKIOVTrW0H7oZijZ/32dL2sEMQUVpcwD5+y+s814V 5W53qJ4bGLPpgmCDz9OeErl4XsMaTbv9P0CnKlSGNBIabruUCbEHWf099ieJMVUita uHzmy/yRlupXsmmWdwFWbD6ZYE2ec+IvOM7L/IhYZsIRix2rvZrLTkbkzY5gTksCKW YXrm/TCggSVSxa1iZhNaDJAVz9A4iCccdKxzhdsD/4x6sckJCuPDX3cwAyWq6S5QZg 53zOn6x/I8lMQ== Received: by mail-yb1-f176.google.com with SMTP id b131so16877015yba.11 for ; Tue, 22 Nov 2022 03:02:30 -0800 (PST) X-Gm-Message-State: ANoB5plsitcDi010buY/55303A+mCVYbjPTARleCZy9X6S3EFopxqnaK bXZ6zTbzI8h9HFa9DiSrr1N3OCaBWnzHgbaPK0s= X-Received: by 2002:a25:cf8d:0:b0:6be:b9e1:8454 with SMTP id f135-20020a25cf8d000000b006beb9e18454mr20512320ybg.94.1669114949504; Tue, 22 Nov 2022 03:02:29 -0800 (PST) MIME-Version: 1.0 References: <20221119204435.97113-1-ogabbay@kernel.org> <20221119204435.97113-4-ogabbay@kernel.org> <20221122105552.edse4v3zb5q23e3k@mail.igalia.com> In-Reply-To: From: Oded Gabbay Date: Tue, 22 Nov 2022 13:02:03 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v4 3/4] drm: initialize accel framework To: Melissa Wen Cc: David Airlie , Daniel Vetter , Greg Kroah-Hartman , Tvrtko Ursulin , Jacek Lawrynowicz , Jeffrey Hugo , Jason Gunthorpe , Arnd Bergmann , Jiho Chu , John Hubbard , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Christoph Hellwig , Christopher Friedt , Thomas Zimmermann , Kevin Hilman , Alex Deucher , Yuji Ishikawa , Maciej Kwapulinski , Jagan Teki Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 22, 2022 at 12:59 PM Oded Gabbay wrote: > > On Tue, Nov 22, 2022 at 12:56 PM Melissa Wen wrote: > > > > On 11/19, Oded Gabbay wrote: > > > Now that we have the accel framework code ready, let's call the > > > accel functions from all the appropriate places. These places are the > > > drm module init/exit functions, and all the drm_minor handling > > > functions. > > > > Hi Oded, > > > > The proposal overall LGTM, but I didn't manage to compile the kernel > > with your patch series when DRM is enabled but accel support doesn't. > > Multiple missing link errors... > > > > Am I missing something? Can you take a look at it from this patch 3/4? > > > > Thanks, > > > > Melissa > Looking at it now, thanks for letting me know. > Oded Found the issue, missing } at end of accel_debugfs_init() in drm_accel.h. Only compiles when accel support isn't compiled in. I'll fix it before sending the PR to Dave. Much appreciated :) Oded > > > > > > > Signed-off-by: Oded Gabbay > > > --- > > > drivers/gpu/drm/drm_drv.c | 102 ++++++++++++++++++++++++++---------- > > > drivers/gpu/drm/drm_sysfs.c | 24 ++++++--- > > > 2 files changed, 91 insertions(+), 35 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > > index 8214a0b1ab7f..1aec019f6389 100644 > > > --- a/drivers/gpu/drm/drm_drv.c > > > +++ b/drivers/gpu/drm/drm_drv.c > > > @@ -35,6 +35,7 @@ > > > #include > > > #include > > > > > > +#include > > > #include > > > #include > > > #include > > > @@ -90,6 +91,8 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev, > > > return &dev->primary; > > > case DRM_MINOR_RENDER: > > > return &dev->render; > > > + case DRM_MINOR_ACCEL: > > > + return &dev->accel; > > > default: > > > BUG(); > > > } > > > @@ -104,9 +107,13 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data) > > > > > > put_device(minor->kdev); > > > > > > - spin_lock_irqsave(&drm_minor_lock, flags); > > > - idr_remove(&drm_minors_idr, minor->index); > > > - spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + if (minor->type == DRM_MINOR_ACCEL) { > > > + accel_minor_remove(minor->index); > > > + } else { > > > + spin_lock_irqsave(&drm_minor_lock, flags); > > > + idr_remove(&drm_minors_idr, minor->index); > > > + spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + } > > > } > > > > > > static int drm_minor_alloc(struct drm_device *dev, unsigned int type) > > > @@ -123,13 +130,17 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type) > > > minor->dev = dev; > > > > > > idr_preload(GFP_KERNEL); > > > - spin_lock_irqsave(&drm_minor_lock, flags); > > > - r = idr_alloc(&drm_minors_idr, > > > - NULL, > > > - 64 * type, > > > - 64 * (type + 1), > > > - GFP_NOWAIT); > > > - spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + if (type == DRM_MINOR_ACCEL) { > > > + r = accel_minor_alloc(); > > > + } else { > > > + spin_lock_irqsave(&drm_minor_lock, flags); > > > + r = idr_alloc(&drm_minors_idr, > > > + NULL, > > > + 64 * type, > > > + 64 * (type + 1), > > > + GFP_NOWAIT); > > > + spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + } > > > idr_preload_end(); > > > > > > if (r < 0) > > > @@ -161,10 +172,14 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type) > > > if (!minor) > > > return 0; > > > > > > - ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root); > > > - if (ret) { > > > - DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n"); > > > - goto err_debugfs; > > > + if (minor->type == DRM_MINOR_ACCEL) { > > > + accel_debugfs_init(minor, minor->index); > > > + } else { > > > + ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root); > > > + if (ret) { > > > + DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n"); > > > + goto err_debugfs; > > > + } > > > } > > > > > > ret = device_add(minor->kdev); > > > @@ -172,9 +187,13 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type) > > > goto err_debugfs; > > > > > > /* replace NULL with @minor so lookups will succeed from now on */ > > > - spin_lock_irqsave(&drm_minor_lock, flags); > > > - idr_replace(&drm_minors_idr, minor, minor->index); > > > - spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + if (minor->type == DRM_MINOR_ACCEL) { > > > + accel_minor_replace(minor, minor->index); > > > + } else { > > > + spin_lock_irqsave(&drm_minor_lock, flags); > > > + idr_replace(&drm_minors_idr, minor, minor->index); > > > + spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + } > > > > > > DRM_DEBUG("new minor registered %d\n", minor->index); > > > return 0; > > > @@ -194,9 +213,13 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type) > > > return; > > > > > > /* replace @minor with NULL so lookups will fail from now on */ > > > - spin_lock_irqsave(&drm_minor_lock, flags); > > > - idr_replace(&drm_minors_idr, NULL, minor->index); > > > - spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + if (minor->type == DRM_MINOR_ACCEL) { > > > + accel_minor_replace(NULL, minor->index); > > > + } else { > > > + spin_lock_irqsave(&drm_minor_lock, flags); > > > + idr_replace(&drm_minors_idr, NULL, minor->index); > > > + spin_unlock_irqrestore(&drm_minor_lock, flags); > > > + } > > > > > > device_del(minor->kdev); > > > dev_set_drvdata(minor->kdev, NULL); /* safety belt */ > > > @@ -603,6 +626,14 @@ static int drm_dev_init(struct drm_device *dev, > > > /* no per-device feature limits by default */ > > > dev->driver_features = ~0u; > > > > > > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) && > > > + (drm_core_check_feature(dev, DRIVER_RENDER) || > > > + drm_core_check_feature(dev, DRIVER_MODESET))) { > > > + > > > + DRM_ERROR("DRM driver can't be both a compute acceleration and graphics driver\n"); > > > + return -EINVAL; > > > + } > > > + > > > drm_legacy_init_members(dev); > > > INIT_LIST_HEAD(&dev->filelist); > > > INIT_LIST_HEAD(&dev->filelist_internal); > > > @@ -628,15 +659,21 @@ static int drm_dev_init(struct drm_device *dev, > > > > > > dev->anon_inode = inode; > > > > > > - if (drm_core_check_feature(dev, DRIVER_RENDER)) { > > > - ret = drm_minor_alloc(dev, DRM_MINOR_RENDER); > > > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) { > > > + ret = drm_minor_alloc(dev, DRM_MINOR_ACCEL); > > > if (ret) > > > goto err; > > > - } > > > + } else { > > > + if (drm_core_check_feature(dev, DRIVER_RENDER)) { > > > + ret = drm_minor_alloc(dev, DRM_MINOR_RENDER); > > > + if (ret) > > > + goto err; > > > + } > > > > > > - ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY); > > > - if (ret) > > > - goto err; > > > + ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY); > > > + if (ret) > > > + goto err; > > > + } > > > > > > ret = drm_legacy_create_map_hash(dev); > > > if (ret) > > > @@ -883,6 +920,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > > > if (ret) > > > goto err_minors; > > > > > > + ret = drm_minor_register(dev, DRM_MINOR_ACCEL); > > > + if (ret) > > > + goto err_minors; > > > + > > > ret = create_compat_control_link(dev); > > > if (ret) > > > goto err_minors; > > > @@ -902,12 +943,13 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > > > driver->name, driver->major, driver->minor, > > > driver->patchlevel, driver->date, > > > dev->dev ? dev_name(dev->dev) : "virtual device", > > > - dev->primary->index); > > > + dev->primary ? dev->primary->index : dev->accel->index); > > > > > > goto out_unlock; > > > > > > err_minors: > > > remove_compat_control_link(dev); > > > + drm_minor_unregister(dev, DRM_MINOR_ACCEL); > > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > > > drm_minor_unregister(dev, DRM_MINOR_RENDER); > > > out_unlock: > > > @@ -950,6 +992,7 @@ void drm_dev_unregister(struct drm_device *dev) > > > drm_legacy_rmmaps(dev); > > > > > > remove_compat_control_link(dev); > > > + drm_minor_unregister(dev, DRM_MINOR_ACCEL); > > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > > > drm_minor_unregister(dev, DRM_MINOR_RENDER); > > > } > > > @@ -1034,6 +1077,7 @@ static const struct file_operations drm_stub_fops = { > > > static void drm_core_exit(void) > > > { > > > drm_privacy_screen_lookup_exit(); > > > + accel_core_exit(); > > > unregister_chrdev(DRM_MAJOR, "drm"); > > > debugfs_remove(drm_debugfs_root); > > > drm_sysfs_destroy(); > > > @@ -1061,6 +1105,10 @@ static int __init drm_core_init(void) > > > if (ret < 0) > > > goto error; > > > > > > + ret = accel_core_init(); > > > + if (ret < 0) > > > + goto error; > > > + > > > drm_privacy_screen_lookup_init(); > > > > > > drm_core_init_complete = true; > > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c > > > index 430e00b16eec..b8da978d85bb 100644 > > > --- a/drivers/gpu/drm/drm_sysfs.c > > > +++ b/drivers/gpu/drm/drm_sysfs.c > > > @@ -19,6 +19,7 @@ > > > #include > > > #include > > > > > > +#include > > > #include > > > #include > > > #include > > > @@ -471,19 +472,26 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor) > > > struct device *kdev; > > > int r; > > > > > > - if (minor->type == DRM_MINOR_RENDER) > > > - minor_str = "renderD%d"; > > > - else > > > - minor_str = "card%d"; > > > - > > > kdev = kzalloc(sizeof(*kdev), GFP_KERNEL); > > > if (!kdev) > > > return ERR_PTR(-ENOMEM); > > > > > > device_initialize(kdev); > > > - kdev->devt = MKDEV(DRM_MAJOR, minor->index); > > > - kdev->class = drm_class; > > > - kdev->type = &drm_sysfs_device_minor; > > > + > > > + if (minor->type == DRM_MINOR_ACCEL) { > > > + minor_str = "accel%d"; > > > + accel_set_device_instance_params(kdev, minor->index); > > > + } else { > > > + if (minor->type == DRM_MINOR_RENDER) > > > + minor_str = "renderD%d"; > > > + else > > > + minor_str = "card%d"; > > > + > > > + kdev->devt = MKDEV(DRM_MAJOR, minor->index); > > > + kdev->class = drm_class; > > > + kdev->type = &drm_sysfs_device_minor; > > > + } > > > + > > > kdev->parent = minor->dev->dev; > > > kdev->release = drm_sysfs_release; > > > dev_set_drvdata(kdev, minor); > > > -- > > > 2.25.1 > > >