Re: [PATCH v2 6/6] drm/vkms: Add a module param to enable/disable the default device

From: Brandon Ross Pollack
Date: Fri Aug 18 2023 - 01:40:17 EST

Next message: Stephen Rothwell: "linux-next: build failure after merge of the phy-next tree"
Previous message: Michael Shavit: "Re: [RFC PATCH v1 1/8] iommu/arm-smmu-v3: Add list of installed_smmus"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 6/26/23 03:04, Maira Canal wrote:

Hi Jim,

On 6/23/23 19:23, Jim Shargo wrote:

In many testing circumstances, we will want to just create a new device
and test against that. If we create a default device, it can be annoying
to have to manually select the new device instead of choosing the only
one that exists.

The param, enable_default, is defaulted to true to maintain backwards
compatibility.

Signed-off-by: Jim Shargo <jshargo@xxxxxxxxxxxx>
---
drivers/gpu/drm/vkms/vkms_drv.c | 44 ++++++++++++++++++++++-----------
1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 314a04659c5f..1cb56c090a65 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -42,17 +42,26 @@
#define DRIVER_MAJOR    1
#define DRIVER_MINOR    0
+static bool enable_default_device = true;
+module_param_named(enable_default_device, enable_default_device, bool, 0444);
+MODULE_PARM_DESC(enable_default_device,
+         "Enable/Disable creating the default device");

Wouldn't be better to just call it "enable_default"?

At the risk of being annoyingly pedantic, I actually like the original name better because it makes it clear it is an entire device and setup,

including the planes/encoders/connectors/crtcs needed and connecting them as necessary etc. I just feel "device" here makes it clearer what is the default thing.

Also, could you add this parameter to vkms_config debugfs file?

But of course! This is the last comment before I send out the new series.

Thank you so much for your time, looking forward to the next round of comments.

Best Regards,
- Maíra

+
static bool enable_cursor = true;
module_param_named(enable_cursor, enable_cursor, bool, 0444);
-MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
+MODULE_PARM_DESC(enable_cursor,
+         "Enable/Disable cursor support for the default device");
static bool enable_writeback = true;
module_param_named(enable_writeback, enable_writeback, bool, 0444);
-MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector support");
+MODULE_PARM_DESC(
+    enable_writeback,
+    "Enable/Disable writeback connector support for the default device");
static bool enable_overlay;
module_param_named(enable_overlay, enable_overlay, bool, 0444);
-MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
+MODULE_PARM_DESC(enable_overlay,
+         "Enable/Disable overlay support for the default device");
DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
@@ -278,10 +287,7 @@ void vkms_remove_device(struct vkms_device *vkms_device)
static int __init vkms_init(void)
{
      int ret;
-    struct platform_device *pdev;
-    struct vkms_device_setup vkms_device_setup = {
-        .configfs = NULL,
-    };
+    struct platform_device *default_pdev = NULL;
      ret = platform_driver_register(&vkms_platform_driver);
      if (ret) {
@@ -289,19 +295,27 @@ static int __init vkms_init(void)
          return ret;
      }
-    pdev = platform_device_register_data(NULL, DRIVER_NAME, 0,
-                         &vkms_device_setup,
-                         sizeof(vkms_device_setup));
-    if (IS_ERR(pdev)) {
-        DRM_ERROR("Unable to register default vkms device\n");
-        platform_driver_unregister(&vkms_platform_driver);
-        return PTR_ERR(pdev);
+    if (enable_default_device) {
+        struct vkms_device_setup vkms_device_setup = {
+            .configfs = NULL,
+        };
+
+        default_pdev = platform_device_register_data(
+            NULL, DRIVER_NAME, 0, &vkms_device_setup,
+            sizeof(vkms_device_setup));
+        if (IS_ERR(default_pdev)) {
+            DRM_ERROR("Unable to register default vkms device\n");
+ platform_driver_unregister(&vkms_platform_driver);
+            return PTR_ERR(default_pdev);
+        }
      }
      ret = vkms_init_configfs();
      if (ret) {
          DRM_ERROR("Unable to initialize configfs\n");
-        platform_device_unregister(pdev);
+        if (default_pdev)
+            platform_device_unregister(default_pdev);
+
          platform_driver_unregister(&vkms_platform_driver);
      }

From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
    aws-us-west-2-korg-lkml-1.web.codeaurora.org
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 smtp.lore.kernel.org (Postfix) with ESMTPS id AB561C0015E
    for <dri-devel@xxxxxxxxxxxxxxxxxxx>; Sun, 25 Jun 2023 18:04:15 +0000 (UTC)
Received: from gabe.freedesktop.org (localhost [127.0.0.1])
    by gabe.freedesktop.org (Postfix) with ESMTP id D153310E18C;
    Sun, 25 Jun 2023 18:04:14 +0000 (UTC)
Received: from mx1.riseup.net (mx1.riseup.net [198.252.153.129])
by gabe.freedesktop.org (Postfix) with ESMTPS id C6BE910E187
for <dri-devel@xxxxxxxxxxxxxxxxxxxxx>; Sun, 25 Jun 2023 18:04:12 +0000 (UTC)
Received: from fews01-sea.riseup.net (fews01-sea-pn.riseup.net [10.0.1.109])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by mx1.riseup.net (Postfix) with ESMTPS id 4QpzPl6CHTzDrNy;
Sun, 25 Jun 2023 18:04:11 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=riseup.net; s=squak;
t=1687716252; bh=3cOd9Tulf+RLZ2R/lGuVfEdERy8xqZ4HHWjkWZv3FAs=;
h=Date:Subject:To:Cc:References:From:In-Reply-To:From;
b=lXentHTOo29YwgA/Q5WAgBBEUsl5DTqsmOJd4wsjxBgZocgZ/PG9hmyBm6a2IjvcQ
jSPI26UWubdYOe7kngWhcU0/oO570ofVUyrxiNgiJQfcscdp5bpwrskBKK4yXdJc0q
2YM4+n0xeBBSr2pFLMDwbOsLDvaGUK77nSPcbPXQ=
X-Riseup-User-ID: 7EBE90DED2258EE1CA830B796CBA05FD63338D890F31F4C1BCCCB27A6DA77A56
Received: from [127.0.0.1] (localhost [127.0.0.1])
by fews01-sea.riseup.net (Postfix) with ESMTPSA id 4QpzPg4YpbzJntB;
Sun, 25 Jun 2023 18:04:07 +0000 (UTC)
Message-ID: <64c40359-d0ee-5070-2a52-033c7e655e0a@xxxxxxxxxx>
Date: Sun, 25 Jun 2023 15:04:05 -0300
MIME-Version: 1.0
Subject: Re: [PATCH v2 6/6] drm/vkms: Add a module param to enable/disable the
default device
Content-Language: en-US
To: Jim Shargo <jshargo@xxxxxxxxxxxx>, Daniel Vetter <daniel@xxxxxxxx>,
David Airlie <airlied@xxxxxxxxx>, Haneen Mohammed <hamohammed.sa@xxxxxxxxx>,
Jonathan Corbet <corbet@xxxxxxx>,
Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>,
Maxime Ripard <mripard@xxxxxxxxxx>, Melissa Wen <melissa.srw@xxxxxxxxx>,
Rodrigo Siqueira <rodrigosiqueiramelo@xxxxxxxxx>,
Thomas Zimmermann <tzimmermann@xxxxxxx>
References: <20230623222353.97283-1-jshargo@xxxxxxxxxxxx>
<20230623222353.97283-7-jshargo@xxxxxxxxxxxx>
From: Maira Canal <mairacanal@xxxxxxxxxx>
In-Reply-To: <20230623222353.97283-7-jshargo@xxxxxxxxxxxx>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
X-BeenThere: dri-devel@xxxxxxxxxxxxxxxxxxxxx
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Direct Rendering Infrastructure - Development
<dri-devel.lists.freedesktop.org>
List-Unsubscribe: <https://lists.freedesktop.org/mailman/options/dri-devel>,
<mailto:dri-devel-request@xxxxxxxxxxxxxxxxxxxxx?subject=unsubscribe>
List-Archive: <https://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@xxxxxxxxxxxxxxxxxxxxx>
List-Help: <mailto:dri-devel-request@xxxxxxxxxxxxxxxxxxxxx?subject=help>
List-Subscribe: <https://lists.freedesktop.org/mailman/listinfo/dri-devel>,
<mailto:dri-devel-request@xxxxxxxxxxxxxxxxxxxxx?subject=subscribe>
Cc: linux-kernel@xxxxxxxxxxxxxxx, dri-devel@xxxxxxxxxxxxxxxxxxxxx,
linux-doc@xxxxxxxxxxxxxxx
Errors-To: dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx
Sender: "dri-devel" <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx>

Hi Jim,

On 6/23/23 19:23, Jim Shargo wrote:

In many testing circumstances, we will want to just create a new device
and test against that. If we create a default device, it can be annoying
to have to manually select the new device instead of choosing the only
one that exists.

The param, enable_default, is defaulted to true to maintain backwards
compatibility.

Signed-off-by: Jim Shargo <jshargo@xxxxxxxxxxxx>
---
drivers/gpu/drm/vkms/vkms_drv.c | 44 ++++++++++++++++++++++-----------
1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 314a04659c5f..1cb56c090a65 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -42,17 +42,26 @@
#define DRIVER_MAJOR    1
#define DRIVER_MINOR    0
+static bool enable_default_device = true;
+module_param_named(enable_default_device, enable_default_device, bool, 0444);
+MODULE_PARM_DESC(enable_default_device,
+         "Enable/Disable creating the default device");

Wouldn't be better to just call it "enable_default"?

Also, could you add this parameter to vkms_config debugfs file?

Best Regards,
- Maíra

+
static bool enable_cursor = true;
module_param_named(enable_cursor, enable_cursor, bool, 0444);
-MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
+MODULE_PARM_DESC(enable_cursor,
+         "Enable/Disable cursor support for the default device");
static bool enable_writeback = true;
module_param_named(enable_writeback, enable_writeback, bool, 0444);
-MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector support");
+MODULE_PARM_DESC(
+    enable_writeback,
+    "Enable/Disable writeback connector support for the default device");
static bool enable_overlay;
module_param_named(enable_overlay, enable_overlay, bool, 0444);
-MODULE_PARM_DESC(enable_overlay, "Enable/Disable overlay support");
+MODULE_PARM_DESC(enable_overlay,
+         "Enable/Disable overlay support for the default device");
DEFINE_DRM_GEM_FOPS(vkms_driver_fops);
@@ -278,10 +287,7 @@ void vkms_remove_device(struct vkms_device *vkms_device)
static int __init vkms_init(void)
{
      int ret;
-    struct platform_device *pdev;
-    struct vkms_device_setup vkms_device_setup = {
-        .configfs = NULL,
-    };
+    struct platform_device *default_pdev = NULL;
      ret = platform_driver_register(&vkms_platform_driver);
      if (ret) {
@@ -289,19 +295,27 @@ static int __init vkms_init(void)
          return ret;
      }
-    pdev = platform_device_register_data(NULL, DRIVER_NAME, 0,
-                         &vkms_device_setup,
-                         sizeof(vkms_device_setup));
-    if (IS_ERR(pdev)) {
-        DRM_ERROR("Unable to register default vkms device\n");
-        platform_driver_unregister(&vkms_platform_driver);
-        return PTR_ERR(pdev);
+    if (enable_default_device) {
+        struct vkms_device_setup vkms_device_setup = {
+            .configfs = NULL,
+        };
+
+        default_pdev = platform_device_register_data(
+            NULL, DRIVER_NAME, 0, &vkms_device_setup,
+            sizeof(vkms_device_setup));
+        if (IS_ERR(default_pdev)) {
+            DRM_ERROR("Unable to register default vkms device\n");
+ platform_driver_unregister(&vkms_platform_driver);
+            return PTR_ERR(default_pdev);
+        }
      }
      ret = vkms_init_configfs();
      if (ret) {
          DRM_ERROR("Unable to initialize configfs\n");
-        platform_device_unregister(pdev);
+        if (default_pdev)
+            platform_device_unregister(default_pdev);
+
          platform_driver_unregister(&vkms_platform_driver);
      }

Next message: Stephen Rothwell: "linux-next: build failure after merge of the phy-next tree"
Previous message: Michael Shavit: "Re: [RFC PATCH v1 1/8] iommu/arm-smmu-v3: Add list of installed_smmus"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]