[{"id":23312,"web_url":"https://patchwork.libcamera.org/comment/23312/","msgid":"","date":"2022-06-03T21:52:46","subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Eric,\n\nThank you for the patch.\n\nOn Thu, Jun 02, 2022 at 11:00:17AM +0100, Eric Curtin via libcamera-devel wrote:\n> Existing code is hardcoded to card0. Since recent fedora upgrades, we\n> have noticed on more than one machine that card1 is present as the\n> lowest numbered device, could theoretically be higher.\n\nOut of curiosity, why is that ?\n\n> This technique\n> tries every file starting with card and continue only when we have\n> successfully opened one. These devices with card1 as the lowest device\n> were simply failing when they do not see a /dev/dri/card0 file present.\n> \n> Reported-by: Ian Mullins \n> Signed-off-by: Eric Curtin \n> \n> ---\n> Changes in v3:\n> - switch back to memcpy, strncpy causing false compiler issue\n> \n> Changes in v2:\n> - return if no valid card found, or directory could not be opened etc.\n> - memcpy to strncpy for safety\n> - only adjust the numerical bytes for each iteration of loop since\n> /dev/dri/card won't change\n> - initialize ret to zero\n> ---\n> src/cam/drm.cpp | 46 ++++++++++++++++++++++++++++++++++++----------\n> 1 file changed, 36 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp\n> index 42c5a3b1..7975a502 100644\n> --- a/src/cam/drm.cpp\n> +++ b/src/cam/drm.cpp\n> @@ -8,6 +8,7 @@\n> #include \"drm.h\"\n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -393,9 +394,13 @@ Device::~Device()\n> \n> int Device::init()\n> {\n> -\tconstexpr size_t NODE_NAME_MAX = sizeof(\"/dev/dri/card255\");\n> -\tchar name[NODE_NAME_MAX];\n> -\tint ret;\n> +\tconstexpr size_t DIR_NAME_MAX = sizeof(\"/dev/dri/\");\n> +\tconstexpr size_t PRE_NODE_NAME_MAX = sizeof(\"card\");\n> +\tconstexpr size_t POST_NODE_NAME_MAX = sizeof(\"255\");\n> +\tconstexpr size_t NODE_NAME_MAX =\n> +\t\tDIR_NAME_MAX + PRE_NODE_NAME_MAX + POST_NODE_NAME_MAX - 2;\n> +\tchar name[NODE_NAME_MAX] = \"/dev/dri/\";\n\nThis looks pretty error-prone compared to std::string, let's use the C++\nAPI here instead. This isn't performance-sensitive code.\n\nstd::filesystem could be nice too, to avoid the manual closedir(), but\nthat's a lesser concern.\n\n> +\tint ret = 0;\n> \n> \t/*\n> \t * Open the first DRM/KMS device. The libdrm drmOpen*() functions\n> @@ -404,14 +409,35 @@ int Device::init()\n> \t * from drmOpen() is of no practical use as any modern system will\n> \t * handle that through udev or an equivalent component.\n> \t */\n> -\tsnprintf(name, sizeof(name), \"/dev/dri/card%u\", 0);\n> -\tfd_ = open(name, O_RDWR | O_CLOEXEC);\n> +\tDIR *folder = opendir(name);\n> +\tmemcpy(name + DIR_NAME_MAX - 1, \"card\", PRE_NODE_NAME_MAX);\n> +\tif (folder) {\n\nYou can turn this into a\n\n\tif (!folder) {\n\t\t...\n\t}\n\nto lower the indentation below.\n\n> +\t\tfor (struct dirent *res; (res = readdir(folder));) {\n> +\t\t\tif (!strncmp(res->d_name, \"card\", 4)) {\n\nSame thing here,\n\n\t\tif (strncmp(...))\n\t\t\tcontinue;\n\n(or rather the equivalent after switching to std::string)\n\n> +\t\t\t\tmemcpy(name + DIR_NAME_MAX +\n> +\t\t\t\t\t\tPRE_NODE_NAME_MAX - 2,\n> +\t\t\t\t\tres->d_name + PRE_NODE_NAME_MAX - 1,\n> +\t\t\t\t\tPOST_NODE_NAME_MAX);\n> +\t\t\t\tfd_ = open(name, O_RDWR | O_CLOEXEC);\n> +\t\t\t\tif (fd_ < 0) {\n> +\t\t\t\t\tret = -errno;\n> +\t\t\t\t\tstd::cerr\n> +\t\t\t\t\t\t<< \"Failed to open DRM/KMS device \"\n> +\t\t\t\t\t\t<< name << \": \"\n> +\t\t\t\t\t\t<< strerror(-ret) << std::endl;\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\t}\n> +\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tclosedir(folder);\n> +\t}\n> +\n> \tif (fd_ < 0) {\n> -\t\tret = -errno;\n> -\t\tstd::cerr\n> -\t\t\t<< \"Failed to open DRM/KMS device \" << name << \": \"\n> -\t\t\t<< strerror(-ret) << std::endl;\n> -\t\treturn ret;\n> +\t\tstd::cerr << \"Unable to open any DRM/KMS device\" << std::endl;\n> +\t\treturn ret ? ret : -ENOENT;\n\nI don't think it's very meaningful to pick the last error here, you can\njust return -ENOENT\n\n> \t}\n\nIt could be interesting to try all available DRM devices to find the\nrequested connector, as well as allowing selection of a particular\ndevice on the command line. That's an improvement candidate for the\nfuture, if a need arises.\n\n> \n> \t/*","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 84BA5BD160\n\tfor ;\n\tFri, 3 Jun 2022 21:52:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B040F65635;\n\tFri, 3 Jun 2022 23:52:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 813D660105\n\tfor ;\n\tFri, 3 Jun 2022 23:52:51 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(lmontsouris-659-1-41-236.w92-154.abo.wanadoo.fr [92.154.76.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1DED9A2A;\n\tFri, 3 Jun 2022 23:52:51 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654293173;\n\tbh=Coj57IOnpptusBwlyxbN6aJaGFFWzt2kAmN0Bss03vo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GRPzovL9vqxksWhDJ30y+NLvFtUvWuHedce1+LxZSygkdlU9TNR/4LN3BReeSC+BU\n\tzDjGwADKdzMN1a/bLQUocDlB11xuYCjmObOSo6ypeBU7AQ5MNTY2c+kt2vYXOHgy6H\n\tEwqE5t9aD0jewjAIoFyDbLmujPKBdKF7JsLIkF691s712HnDypNnEx0MCBKHe6dVpL\n\tHdBXedclEiuAIGHeSM4mTdp2U7GDo70Fv2//X9tqziDd103KEkGh+HBt9QHGihATj2\n\t08Jf767Z9zfR+a3fiqivj40WYjwWvUDD+Fq1Z1ctLjot7bhHBp4jbbBtoe99VrhxnE\n\tKgvB/lztnWIxQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654293171;\n\tbh=Coj57IOnpptusBwlyxbN6aJaGFFWzt2kAmN0Bss03vo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VkH/dv6GOIc6ZGPgHxx0JcPzImLfy6Yb7yFV16GATE5NImIwhn8rEtSwYzkbDqlG3\n\tDcpnPlRfY9yQ1POpOKKNIC6irR/f7nJ/UW1cyVmdUeHaTLflBXKAD3h5AGSG3bxp36\n\tVMEaX85i6yjmTpjAzNWODE5YwDIU4JX09iUI+d4M="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VkH/dv6G\"; dkim-atps=neutral","Date":"Sat, 4 Jun 2022 00:52:46 +0300","To":"Eric Curtin ","Message-ID":"","References":"<20220602100017.22272-1-ecurtin@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220602100017.22272-1-ecurtin@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Ian Mullins , libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23327,"web_url":"https://patchwork.libcamera.org/comment/23327/","msgid":"","date":"2022-06-05T15:25:50","subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","submitter":{"id":101,"url":"https://patchwork.libcamera.org/api/people/101/","name":"Eric Curtin","email":"ecurtin@redhat.com"},"content":"On Fri, 3 Jun 2022 at 22:52, Laurent Pinchart\n wrote:\n>\n> Hi Eric,\n>\n> Thank you for the patch.\n>\n> On Thu, Jun 02, 2022 at 11:00:17AM +0100, Eric Curtin via libcamera-devel wrote:\n> > Existing code is hardcoded to card0. Since recent fedora upgrades, we\n> > have noticed on more than one machine that card1 is present as the\n> > lowest numbered device, could theoretically be higher.\n>\n> Out of curiosity, why is that ?\n\nI don't know, I didn't mind too much, because this patch fixed it and\nit seemed like the more correct way anyway. CC'ing Javier, just in\ncase he knows, being a DRM guru. I've seen it on 2 machines I own that\nupdated recently and Ian also saw it on his machine.\n\n>\n> > This technique\n> > tries every file starting with card and continue only when we have\n> > successfully opened one. These devices with card1 as the lowest device\n> > were simply failing when they do not see a /dev/dri/card0 file present.\n> >\n> > Reported-by: Ian Mullins \n> > Signed-off-by: Eric Curtin \n> >\n> > ---\n> > Changes in v3:\n> > - switch back to memcpy, strncpy causing false compiler issue\n> >\n> > Changes in v2:\n> > - return if no valid card found, or directory could not be opened etc.\n> > - memcpy to strncpy for safety\n> > - only adjust the numerical bytes for each iteration of loop since\n> > /dev/dri/card won't change\n> > - initialize ret to zero\n> > ---\n> > src/cam/drm.cpp | 46 ++++++++++++++++++++++++++++++++++++----------\n> > 1 file changed, 36 insertions(+), 10 deletions(-)\n> >\n> > diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp\n> > index 42c5a3b1..7975a502 100644\n> > --- a/src/cam/drm.cpp\n> > +++ b/src/cam/drm.cpp\n> > @@ -8,6 +8,7 @@\n> > #include \"drm.h\"\n> >\n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -393,9 +394,13 @@ Device::~Device()\n> >\n> > int Device::init()\n> > {\n> > - constexpr size_t NODE_NAME_MAX = sizeof(\"/dev/dri/card255\");\n> > - char name[NODE_NAME_MAX];\n> > - int ret;\n> > + constexpr size_t DIR_NAME_MAX = sizeof(\"/dev/dri/\");\n> > + constexpr size_t PRE_NODE_NAME_MAX = sizeof(\"card\");\n> > + constexpr size_t POST_NODE_NAME_MAX = sizeof(\"255\");\n> > + constexpr size_t NODE_NAME_MAX =\n> > + DIR_NAME_MAX + PRE_NODE_NAME_MAX + POST_NODE_NAME_MAX - 2;\n> > + char name[NODE_NAME_MAX] = \"/dev/dri/\";\n>\n> This looks pretty error-prone compared to std::string, let's use the C++\n> API here instead. This isn't performance-sensitive code.\n\nWill std::stringify it.\n\n>\n> std::filesystem could be nice too, to avoid the manual closedir(), but\n> that's a lesser concern.\n>\n> > + int ret = 0;\n> >\n> > /*\n> > * Open the first DRM/KMS device. The libdrm drmOpen*() functions\n> > @@ -404,14 +409,35 @@ int Device::init()\n> > * from drmOpen() is of no practical use as any modern system will\n> > * handle that through udev or an equivalent component.\n> > */\n> > - snprintf(name, sizeof(name), \"/dev/dri/card%u\", 0);\n> > - fd_ = open(name, O_RDWR | O_CLOEXEC);\n> > + DIR *folder = opendir(name);\n> > + memcpy(name + DIR_NAME_MAX - 1, \"card\", PRE_NODE_NAME_MAX);\n> > + if (folder) {\n>\n> You can turn this into a\n>\n> if (!folder) {\n> ...\n> }\n>\n> to lower the indentation below.\n\nOk\n\n>\n> > + for (struct dirent *res; (res = readdir(folder));) {\n> > + if (!strncmp(res->d_name, \"card\", 4)) {\n>\n> Same thing here,\n>\n> if (strncmp(...))\n> continue;\n>\n> (or rather the equivalent after switching to std::string)\n>\n> > + memcpy(name + DIR_NAME_MAX +\n> > + PRE_NODE_NAME_MAX - 2,\n> > + res->d_name + PRE_NODE_NAME_MAX - 1,\n> > + POST_NODE_NAME_MAX);\n> > + fd_ = open(name, O_RDWR | O_CLOEXEC);\n> > + if (fd_ < 0) {\n> > + ret = -errno;\n> > + std::cerr\n> > + << \"Failed to open DRM/KMS device \"\n> > + << name << \": \"\n> > + << strerror(-ret) << std::endl;\n> > + continue;\n> > + }\n> > +\n> > + break;\n> > + }\n> > + }\n> > +\n> > + closedir(folder);\n> > + }\n> > +\n> > if (fd_ < 0) {\n> > - ret = -errno;\n> > - std::cerr\n> > - << \"Failed to open DRM/KMS device \" << name << \": \"\n> > - << strerror(-ret) << std::endl;\n> > - return ret;\n> > + std::cerr << \"Unable to open any DRM/KMS device\" << std::endl;\n> > + return ret ? ret : -ENOENT;\n>\n> I don't think it's very meaningful to pick the last error here, you can\n> just return -ENOENT\n\nOk.\n\n>\n> > }\n>\n> It could be interesting to try all available DRM devices to find the\n> requested connector, as well as allowing selection of a particular\n> device on the command line. That's an improvement candidate for the\n> future, if a need arises.\n>\n> >\n> > /*\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D45D5BD161\n\tfor ;\n\tSun, 5 Jun 2022 15:26:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 690F465634;\n\tSun, 5 Jun 2022 17:26:11 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9C4F60104\n\tfor ;\n\tSun, 5 Jun 2022 17:26:09 +0200 (CEST)","from mail-qt1-f198.google.com (mail-qt1-f198.google.com\n\t[209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n\tus-mta-486-anJrSCNAMYWxAK2Pq3YEPw-1; Sun, 05 Jun 2022 11:26:07 -0400","by mail-qt1-f198.google.com with SMTP id\n\tv17-20020a05622a131100b00304c6fc3ab4so9644695qtk.16\n\tfor ;\n\tSun, 05 Jun 2022 08:26:07 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654442771;\n\tbh=1wJZ4MSskv8ow6N9/KxjsQk2Q04rAnl3LJ+PL0OkhG4=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GgpvSDOm7skuy7ziBEI5CXuhpNtK65kfrbKhd5TubQL6a6GR/xNojrhRnbwyZ85yG\n\t+PyY+I5HMGMjiS1ncFNoEEJsfI+pUFpQTF25Z+wtaH5aRAYIUJ+sI8RL6sWiPpLTtu\n\tfJMrA0uqiCIJy6cxC2W6gU0o1M844nUqXev2eJScc/llpL5w9vMWDIV2EzdV+LRaKQ\n\tct1TzMZB6OkgLWBdXQAH3dRObRFheVnBDXugUfCW77qywqTs5YUPl2fQRnwGvOjfIL\n\tyL86nKKn4m6wadOWdZVKpbR9NDxm4TpFkmHmwWJF04qxMuPyDs7NmKSVmeMY9xHdkg\n\tmWAwUKrIVxTOg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1654442768;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=TRd5W6GMYLByblc7RpG3k+xR7m1XbH0LdFSDXxapNkc=;\n\tb=ghoqQxBH98pCkP5PfmYzgLZjv8LBV+ky1tNeuDNsN9p3T/rmQTutZO6DkQeEgmlrNq3Nh7\n\tUlvD4HbAxhm1/iQ8vYRPOsbpkvDV93xHTZOpv572GzrIYGjIhiuStmnTiMx+kmhNlkD+wx\n\tTCVBiw/vtEMKZU7CVcZPi7VuaXWQhJ8="],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=redhat.com\n\theader.i=@redhat.com header.b=\"ghoqQxBH\"; \n\tdkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ecurtin@redhat.com"],"X-MC-Unique":"anJrSCNAMYWxAK2Pq3YEPw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=TRd5W6GMYLByblc7RpG3k+xR7m1XbH0LdFSDXxapNkc=;\n\tb=NjsVlx/u7vnB4tYznK7VHcxnz0xun7uwfcisgo9Ro2x4LZrB2ufjeTO5lGxXwCUnnq\n\tXH1U2lW49u5Qw9YOom1qGuTEvGbfiQULAM+obfPk22VOIy0Kf1oRhOW5eafIfmTeD5UU\n\tCsagTrFS+ws68F2Clq4eotxqRa+zRtPBOFVi2jQo6WtVQH6trA9+TziYq0IPRKkVithF\n\tpsutbQtUpfULyFE/TJGhLH9VnH1bFAtSuxBfTMcOcQGJXu36kH6kjtgi0pdKztOTDS/V\n\tysuYKCXNjCvRFS10b7ET+9G8wmwz4u1cppCADzQ7jMy1OUsfZGmZGZ64ApvYg28nFJRU\n\t7Jrw==","X-Gm-Message-State":"AOAM533jqWs/R6Zvy2LXN5X8431aJcH9zttQBM44HaULbbV1MEfbnSJR\n\tz4Q0LN7ecZJEa0cRoaKqUgvWgcQTX/G6H+BR0zonYlsVuYEzS13YOXbaxgTz8CFk6dy21HIJU4x\n\tjh68px6tYlBj7e5Q9MIPXkCf9r1NvZgO56iIE890lJKCPb0VrMA==","X-Received":["by 2002:ac8:5f48:0:b0:304:c1bf:6dcb with SMTP id\n\ty8-20020ac85f48000000b00304c1bf6dcbmr15398138qta.73.1654442766858; \n\tSun, 05 Jun 2022 08:26:06 -0700 (PDT)","by 2002:ac8:5f48:0:b0:304:c1bf:6dcb with SMTP id\n\ty8-20020ac85f48000000b00304c1bf6dcbmr15398129qta.73.1654442766580;\n\tSun, 05 Jun 2022 08:26:06 -0700 (PDT)"],"X-Google-Smtp-Source":"ABdhPJw/G3d067n2uZXzHIBgRvpGYJINU+cxjDug0jox4G5rzE9VdITdWAXsWWiwz7AHHu3OnK8qWzc0GuQCViKM1YU=","MIME-Version":"1.0","References":"<20220602100017.22272-1-ecurtin@redhat.com>\n\t","In-Reply-To":"","Date":"Sun, 5 Jun 2022 16:25:50 +0100","Message-ID":"","To":"Laurent Pinchart ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Eric Curtin via libcamera-devel ","Reply-To":"Eric Curtin ","Cc":"Ian Mullins ,\n\tJavier Martinez Canillas ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23371,"web_url":"https://patchwork.libcamera.org/comment/23371/","msgid":"","date":"2022-06-09T16:13:25","subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","submitter":{"id":101,"url":"https://patchwork.libcamera.org/api/people/101/","name":"Eric Curtin","email":"ecurtin@redhat.com"},"content":"On Sun, 5 Jun 2022 at 16:25, Eric Curtin wrote:\n>\n> On Fri, 3 Jun 2022 at 22:52, Laurent Pinchart\n> wrote:\n> >\n> > Hi Eric,\n> >\n> > Thank you for the patch.\n> >\n> > On Thu, Jun 02, 2022 at 11:00:17AM +0100, Eric Curtin via libcamera-devel wrote:\n> > > Existing code is hardcoded to card0. Since recent fedora upgrades, we\n> > > have noticed on more than one machine that card1 is present as the\n> > > lowest numbered device, could theoretically be higher.\n> >\n> > Out of curiosity, why is that ?\n>\n> I don't know, I didn't mind too much, because this patch fixed it and\n> it seemed like the more correct way anyway. CC'ing Javier, just in\n> case he knows, being a DRM guru. I've seen it on 2 machines I own that\n> updated recently and Ian also saw it on his machine.\n\nI got an answer on this, it is believed on modern kernels, simpledrm\nwill register as card0, then the real hardware-accelerated driver will\nregister as card1 which will cause simpledrm driver (or card0) to be\nkicked out. Regardless, this was seen as a userspace bug as it should\nnot be assumed that if there is one card registered that it is card0,\nthere are no guarantees here.\n\nCoincidentally I did try this kmssink with a simpledrm driver on\nraspberry pi and it didn't work, while SDL does (which is a kms sink\nin itself when ran without desktop environment running). Never got\naround to figuring out why.\n\n>\n> >\n> > > This technique\n> > > tries every file starting with card and continue only when we have\n> > > successfully opened one. These devices with card1 as the lowest device\n> > > were simply failing when they do not see a /dev/dri/card0 file present.\n> > >\n> > > Reported-by: Ian Mullins \n> > > Signed-off-by: Eric Curtin \n> > >\n> > > ---\n> > > Changes in v3:\n> > > - switch back to memcpy, strncpy causing false compiler issue\n> > >\n> > > Changes in v2:\n> > > - return if no valid card found, or directory could not be opened etc.\n> > > - memcpy to strncpy for safety\n> > > - only adjust the numerical bytes for each iteration of loop since\n> > > /dev/dri/card won't change\n> > > - initialize ret to zero\n> > > ---\n> > > src/cam/drm.cpp | 46 ++++++++++++++++++++++++++++++++++++----------\n> > > 1 file changed, 36 insertions(+), 10 deletions(-)\n> > >\n> > > diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp\n> > > index 42c5a3b1..7975a502 100644\n> > > --- a/src/cam/drm.cpp\n> > > +++ b/src/cam/drm.cpp\n> > > @@ -8,6 +8,7 @@\n> > > #include \"drm.h\"\n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -393,9 +394,13 @@ Device::~Device()\n> > >\n> > > int Device::init()\n> > > {\n> > > - constexpr size_t NODE_NAME_MAX = sizeof(\"/dev/dri/card255\");\n> > > - char name[NODE_NAME_MAX];\n> > > - int ret;\n> > > + constexpr size_t DIR_NAME_MAX = sizeof(\"/dev/dri/\");\n> > > + constexpr size_t PRE_NODE_NAME_MAX = sizeof(\"card\");\n> > > + constexpr size_t POST_NODE_NAME_MAX = sizeof(\"255\");\n> > > + constexpr size_t NODE_NAME_MAX =\n> > > + DIR_NAME_MAX + PRE_NODE_NAME_MAX + POST_NODE_NAME_MAX - 2;\n> > > + char name[NODE_NAME_MAX] = \"/dev/dri/\";\n> >\n> > This looks pretty error-prone compared to std::string, let's use the C++\n> > API here instead. This isn't performance-sensitive code.\n>\n> Will std::stringify it.\n>\n> >\n> > std::filesystem could be nice too, to avoid the manual closedir(), but\n> > that's a lesser concern.\n> >\n> > > + int ret = 0;\n> > >\n> > > /*\n> > > * Open the first DRM/KMS device. The libdrm drmOpen*() functions\n> > > @@ -404,14 +409,35 @@ int Device::init()\n> > > * from drmOpen() is of no practical use as any modern system will\n> > > * handle that through udev or an equivalent component.\n> > > */\n> > > - snprintf(name, sizeof(name), \"/dev/dri/card%u\", 0);\n> > > - fd_ = open(name, O_RDWR | O_CLOEXEC);\n> > > + DIR *folder = opendir(name);\n> > > + memcpy(name + DIR_NAME_MAX - 1, \"card\", PRE_NODE_NAME_MAX);\n> > > + if (folder) {\n> >\n> > You can turn this into a\n> >\n> > if (!folder) {\n> > ...\n> > }\n> >\n> > to lower the indentation below.\n>\n> Ok\n>\n> >\n> > > + for (struct dirent *res; (res = readdir(folder));) {\n> > > + if (!strncmp(res->d_name, \"card\", 4)) {\n> >\n> > Same thing here,\n> >\n> > if (strncmp(...))\n> > continue;\n> >\n> > (or rather the equivalent after switching to std::string)\n> >\n> > > + memcpy(name + DIR_NAME_MAX +\n> > > + PRE_NODE_NAME_MAX - 2,\n> > > + res->d_name + PRE_NODE_NAME_MAX - 1,\n> > > + POST_NODE_NAME_MAX);\n> > > + fd_ = open(name, O_RDWR | O_CLOEXEC);\n> > > + if (fd_ < 0) {\n> > > + ret = -errno;\n> > > + std::cerr\n> > > + << \"Failed to open DRM/KMS device \"\n> > > + << name << \": \"\n> > > + << strerror(-ret) << std::endl;\n> > > + continue;\n> > > + }\n> > > +\n> > > + break;\n> > > + }\n> > > + }\n> > > +\n> > > + closedir(folder);\n> > > + }\n> > > +\n> > > if (fd_ < 0) {\n> > > - ret = -errno;\n> > > - std::cerr\n> > > - << \"Failed to open DRM/KMS device \" << name << \": \"\n> > > - << strerror(-ret) << std::endl;\n> > > - return ret;\n> > > + std::cerr << \"Unable to open any DRM/KMS device\" << std::endl;\n> > > + return ret ? ret : -ENOENT;\n> >\n> > I don't think it's very meaningful to pick the last error here, you can\n> > just return -ENOENT\n>\n> Ok.\n>\n> >\n> > > }\n> >\n> > It could be interesting to try all available DRM devices to find the\n> > requested connector, as well as allowing selection of a particular\n> > device on the command line. That's an improvement candidate for the\n> > future, if a need arises.\n> >\n> > >\n> > > /*\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F3096BD161\n\tfor ;\n\tThu, 9 Jun 2022 16:14:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CD0465635;\n\tThu, 9 Jun 2022 18:14:01 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D92C6559A\n\tfor ;\n\tThu, 9 Jun 2022 18:14:00 +0200 (CEST)","from mail-qk1-f198.google.com (mail-qk1-f198.google.com\n\t[209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n\tus-mta-422-KARPR3gaOAGPrtIVScm88w-1; Thu, 09 Jun 2022 12:13:58 -0400","by mail-qk1-f198.google.com with SMTP id\n\tk13-20020a05620a414d00b006a6e4dc1dfcso6270852qko.19\n\tfor ;\n\tThu, 09 Jun 2022 09:13:58 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654791241;\n\tbh=xo1F2dRhmgfwEdSVBglfwUs+lsPCulp2UMbNjzXNxro=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=HiisupULr+mSC5f8WQSAnAnjywaLsM7cD0XEtZywFD7pF7w6mGyl7qI/wotpd6BC2\n\t1g7LQIdHImXL7/0HD3ygHOz/LZBeo2aR+hPFO1WmMzuQ66c9DKLc/u6F0DAxggE0dX\n\tpPC2ts3t03sV1NXx7sF2Sz9e1NA/9J/U476YwRzfqD2ap2R30dX9UEGyo+f2sAzbD1\n\ta6epaPbuYa0S+pGvD/jniegJBKIH9zxSpQa9rZASiMm6NSkG2fa3yTjrZnNStnuirO\n\tlyHOk9sB4tD+fhGctIZMZMVkdOgK22MRytdNHQqvopRBpC6/V1CHistPWcrkdqdJRw\n\tpWzcF9thZQeOA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1654791239;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=5D09BMCFoK9+a5EKBdjV7GJ/haf/WwUIz++ksly81Sw=;\n\tb=Wee4DDtnatzkfXPT5pA8nmYjZlLvS8iFauymEMT+Q1+yED0Kr4js4yUL0mrZS2euvWWYmb\n\t5+6idym+eOBDTB+t660qetnsWcS1GSWsNbcbDT68sUakAdj0RA94EWz5gfJ4QSFDx2FnDs\n\ttlWGohtzET4KUKtwMVBgBvTRj90dFjc="],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=redhat.com\n\theader.i=@redhat.com header.b=\"Wee4DDtn\"; \n\tdkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ecurtin@redhat.com"],"X-MC-Unique":"KARPR3gaOAGPrtIVScm88w-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=5D09BMCFoK9+a5EKBdjV7GJ/haf/WwUIz++ksly81Sw=;\n\tb=NFJcqM4Xrs3wcUkmZAbf7dvVJxrPnCAwbZYB0ImBKnZrgcjdtLpg/xvYvt6wWJGfrb\n\t4sZMJKvF/zp9m6PZyldfAR3cFDyp4gNITWgnrWBVN8JWpIfX6Gcl6fWIUx8xqLLkHAXB\n\tXRoqXeNnbknjJ0Zja5cx9WtS/HgEtbaaOkPRMZsYTGb5WfM70wbRcZ0irvkYSzEIYg6m\n\teREhXYrqruU+/WFEFBK/hEWOVwHhNH3tt7d4Gn3VVxai683yZ4D1u/bv9wcdaP6AstOA\n\tOPRZV3pzN8IrVA/w0PpilcBCnTY1IgA8xqOtMrBXJu/y4TQk17lKr+Qux4GqCrw+VtIM\n\t4MbA==","X-Gm-Message-State":"AOAM530zaTa9y0MkOE6Ia6yq06QAQ2XcBBsWMKIz3np7n8XT5ZO7zs6x\n\tUdHp3fnu3ZN2Bnh1hUzS97ehLy/nHsjI24SBSYTdmKMRWb4a2jnWlE76yaQ3K8VehBFQLHcCciJ\n\tmKK3rMyj7jNS5Aii5iyZljQZb00v1nqR2PVy6y2M046dOxeUphQ==","X-Received":["by 2002:a05:622a:20b:b0:2f9:1bea:b649 with SMTP id\n\tb11-20020a05622a020b00b002f91beab649mr32620576qtx.301.1654791221861; \n\tThu, 09 Jun 2022 09:13:41 -0700 (PDT)","by 2002:a05:622a:20b:b0:2f9:1bea:b649 with SMTP id\n\tb11-20020a05622a020b00b002f91beab649mr32620538qtx.301.1654791221450;\n\tThu, 09 Jun 2022 09:13:41 -0700 (PDT)"],"X-Google-Smtp-Source":"ABdhPJwE8W5LvB13B604TvMOYTRLUrweRW2Fc7lPGFWSoKrAjrgsBlNPCu5n/SBiEalRtatbKsKlWblakO2X6maIfrk=","MIME-Version":"1.0","References":"<20220602100017.22272-1-ecurtin@redhat.com>\n\t\n\t","In-Reply-To":"","Date":"Thu, 9 Jun 2022 17:13:25 +0100","Message-ID":"","To":"Laurent Pinchart ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Eric Curtin via libcamera-devel ","Reply-To":"Eric Curtin ","Cc":"Ian Mullins ,\n\tJavier Martinez Canillas ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23373,"web_url":"https://patchwork.libcamera.org/comment/23373/","msgid":"<610a8ad4-4023-3ed9-b87f-a44bc2aa3bd0@redhat.com>","date":"2022-06-09T17:59:58","subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","submitter":{"id":95,"url":"https://patchwork.libcamera.org/api/people/95/","name":"Javier Martinez Canillas","email":"javierm@redhat.com"},"content":"Hello,\n\nOn 6/9/22 18:13, Eric Curtin wrote:\n\n[snip]\n\n>>>> Existing code is hardcoded to card0. Since recent fedora upgrades, we\n>>>> have noticed on more than one machine that card1 is present as the\n>>>> lowest numbered device, could theoretically be higher.\n>>>\n>>> Out of curiosity, why is that ?\n>>\n>> I don't know, I didn't mind too much, because this patch fixed it and\n>> it seemed like the more correct way anyway. CC'ing Javier, just in\n>> case he knows, being a DRM guru. I've seen it on 2 machines I own that\n>> updated recently and Ian also saw it on his machine.\n> \n> I got an answer on this, it is believed on modern kernels, simpledrm\n> will register as card0, then the real hardware-accelerated driver will\n> register as card1 which will cause simpledrm driver (or card0) to be\n> kicked out. Regardless, this was seen as a userspace bug as it should\n> not be assumed that if there is one card registered that it is card0,\n> there are no guarantees here.\n> \n\nTo elaborate on this, before F36 we used the {vesa,efi,simple}fb drivers to\nhave early video output but since F36 those have been replaced by simpledrm\nthat as any DRM driver registers a DRI device (as card0 since is the first\ndriver probed due being built-in the kernel).\n\nLater, a real DRM driver is probed and registers its own DRI device. These\nshould remove all conflicting framebuffers when probed, but not all drivers\ndo this at the same time.\n\nFor example, the i915 driver first calls to devm_drm_dev_alloc() function [0]\nand then calls to drm_aperture_remove_conflicting_framebuffers() [1]. There's\na small window then where one has two DRI devices between the time when i915\nregisters its device and the DRM core kicks out the simpledrm driver.\n\nOther DRM drivers call drm_aperture_remove_conflicting_framebuffers() at the\nbeginning of their probe function so there will only be a single DRI device\nat any point.\n\nI asked the DRM folks if they would accept a patch to move the conflicting FB\nremoval earlier but they said that's a user-space bug as Eric mentioned, since\nis a fragile assumption to make anyways.\n\n[0]: https://docs.kernel.org/gpu/drm-internals.html?highlight=devm_drm_dev_alloc#c.devm_drm_dev_alloc\n[1]: https://docs.kernel.org/gpu/drm-internals.html?highlight=remove_conflicting_framebuffers#c.drm_aperture_remove_conflicting_framebuffers","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6410EBD161\n\tfor ;\n\tThu, 9 Jun 2022 18:00:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7301865635;\n\tThu, 9 Jun 2022 20:00:07 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 639C16559A\n\tfor ;\n\tThu, 9 Jun 2022 20:00:04 +0200 (CEST)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n\tus-mta-78-rzn9QpdWODafXopuiIhYqQ-1; Thu, 09 Jun 2022 14:00:02 -0400","by mail-wm1-f72.google.com with SMTP id\n\tk5-20020a05600c0b4500b003941ca130f9so8535211wmr.0\n\tfor ;\n\tThu, 09 Jun 2022 11:00:01 -0700 (PDT)","from [192.168.1.129] (205.pool92-176-231.dynamic.orange.es.\n\t[92.176.231.205]) by smtp.gmail.com with ESMTPSA id\n\tl4-20020a5d5604000000b0020fffbc122asm29810658wrv.99.2022.06.09.10.59.59\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tThu, 09 Jun 2022 10:59:59 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654797607;\n\tbh=RdOFkaG9W0zn9JIz2hqkiWfTaua1IFyyRBzB+1MtJus=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=e1rJdjyHf3HxKMiEQ05j7AjiKDT9pJdDzWNB7pao23Pxq00GA+mMyc2H6MMYbU1BH\n\tQCN4qW0AzaxOJaQVMHGRxiI7ex808UXTJesM9uEiFCiXRHLG54of6IgYKC0TXZ7fmE\n\tv5/dK65fiOoZ2XEPMw8t2DvcWVDNDPdNP9r2jqbk9ojrp0AZ3tmTRK9NmYkVc8tw3i\n\t1WIH3NRaHjGNrX6stvz+yeC7Aow3ery/3WQAC8eoTrpqGE8yh3YSoDLnPqFMedFeBM\n\tjnuAxmfv3MyUZgYNc+Qq0JPbGi3Z6O6L3ltpAH+GJtzpwtx4kbHR7n3Wwu+MqifJYx\n\t1xM3gCDgd0ABA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1654797603;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=BaU9hPGBzKUQcjiTVzLu/KvVfqyqQU0VRllquhBA/0Q=;\n\tb=V05AFmZKDxFokW8Mtbo9bLZ89uAltwgDLiue6SoMeX6q1+CeIL0dP80W8TA1c2ud8ffqhg\n\tICZpRAmF0zA1cR1HdIpwhj9j/JUHabmMwwI3sQD0i9bvnFIKK0Hm7UUYzvFbra6SerrejN\n\tHWvy2si1WSyaG9LzmpLuwF23FF8XE3U="],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=redhat.com\n\theader.i=@redhat.com header.b=\"V05AFmZK\"; \n\tdkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=javierm@redhat.com"],"X-MC-Unique":"rzn9QpdWODafXopuiIhYqQ-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:date:mime-version:user-agent:subject\n\t:content-language:to:cc:references:from:in-reply-to\n\t:content-transfer-encoding;\n\tbh=BaU9hPGBzKUQcjiTVzLu/KvVfqyqQU0VRllquhBA/0Q=;\n\tb=rIce+G1F7IE4uwYCgN4i9CkvZqpI/cfQ6xNCaz1ulF3kR8wiRXEzH5vtpCduvkiG54\n\tJNpkBZJcft+NLdoIlnYslaFk+V3RDKFR7mTDYTzcEg03Lv9zXDh2bLvpBfVOoSDtEQAw\n\ttRv7SHitar4j2MRQrYvUZQ/Ts1zoBPTfEIlMupGo4MPKjp1U/mN6LpPZr3oArur0Qnh3\n\tODlpR574OQERQ1eJ9uRFY1Gw+ZlTaRcAMg2X5niODcHRgjP1L/mHY+81EeOI/MK17ptY\n\t885NKYUzf1SFk762XkB7ZOgEyZBsHscUyaqkij7WybFt5oTxcI8cPgmRM3/aSkFxphit\n\tB4PA==","X-Gm-Message-State":"AOAM531y5hzHtlC1e+JIFNDjNLSnDQowM90prRZS1YUjeJp0G+90l9Sg\n\txgytmgW0vlm0vGX2hQ6rsqKQbxh1b0f8allAjLM2INwuescWY+dEwExvIbegs3f+SnZjxDXUKax\n\tck9PY3cfw8NaBWso9OguL8+Q81aq8D10gqw==","X-Received":["by 2002:a5d:5343:0:b0:210:2f76:649f with SMTP id\n\tt3-20020a5d5343000000b002102f76649fmr38793036wrv.554.1654797600640; \n\tThu, 09 Jun 2022 11:00:00 -0700 (PDT)","by 2002:a5d:5343:0:b0:210:2f76:649f with SMTP id\n\tt3-20020a5d5343000000b002102f76649fmr38793014wrv.554.1654797600294; \n\tThu, 09 Jun 2022 11:00:00 -0700 (PDT)"],"X-Google-Smtp-Source":"ABdhPJy1t9kERe5mPhCkTgsg0/ZqsJZJD8Bb9G28AWEO7oYDmKp10bEYNgJxPbsZ2uf71AoD6xhYVQ==","Message-ID":"<610a8ad4-4023-3ed9-b87f-a44bc2aa3bd0@redhat.com>","Date":"Thu, 9 Jun 2022 19:59:58 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.10.0","To":"Eric Curtin ,\n\tLaurent Pinchart ","References":"<20220602100017.22272-1-ecurtin@redhat.com>\n\t\n\t\n\t","In-Reply-To":"","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Language":"en-US","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Javier Martinez Canillas via libcamera-devel\n\t","Reply-To":"Javier Martinez Canillas ","Cc":"Ian Mullins ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23374,"web_url":"https://patchwork.libcamera.org/comment/23374/","msgid":"","date":"2022-06-09T18:17:15","subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Javier and Eric,\n\nOn Thu, Jun 09, 2022 at 07:59:58PM +0200, Javier Martinez Canillas wrote:\n> On 6/9/22 18:13, Eric Curtin wrote:\n> \n> [snip]\n> \n> >>>> Existing code is hardcoded to card0. Since recent fedora upgrades, we\n> >>>> have noticed on more than one machine that card1 is present as the\n> >>>> lowest numbered device, could theoretically be higher.\n> >>>\n> >>> Out of curiosity, why is that ?\n> >>\n> >> I don't know, I didn't mind too much, because this patch fixed it and\n> >> it seemed like the more correct way anyway. CC'ing Javier, just in\n> >> case he knows, being a DRM guru. I've seen it on 2 machines I own that\n> >> updated recently and Ian also saw it on his machine.\n> > \n> > I got an answer on this, it is believed on modern kernels, simpledrm\n> > will register as card0, then the real hardware-accelerated driver will\n> > register as card1 which will cause simpledrm driver (or card0) to be\n> > kicked out. Regardless, this was seen as a userspace bug as it should\n> > not be assumed that if there is one card registered that it is card0,\n> > there are no guarantees here.\n> > \n> \n> To elaborate on this, before F36 we used the {vesa,efi,simple}fb drivers to\n> have early video output but since F36 those have been replaced by simpledrm\n> that as any DRM driver registers a DRI device (as card0 since is the first\n> driver probed due being built-in the kernel).\n> \n> Later, a real DRM driver is probed and registers its own DRI device. These\n> should remove all conflicting framebuffers when probed, but not all drivers\n> do this at the same time.\n> \n> For example, the i915 driver first calls to devm_drm_dev_alloc() function [0]\n> and then calls to drm_aperture_remove_conflicting_framebuffers() [1]. There's\n> a small window then where one has two DRI devices between the time when i915\n> registers its device and the DRM core kicks out the simpledrm driver.\n> \n> Other DRM drivers call drm_aperture_remove_conflicting_framebuffers() at the\n> beginning of their probe function so there will only be a single DRI device\n> at any point.\n> \n> I asked the DRM folks if they would accept a patch to move the conflicting FB\n> removal earlier but they said that's a user-space bug as Eric mentioned, since\n> is a fragile assumption to make anyways.\n\nThank you for the information. This makes sense.\n\n> [0]: https://docs.kernel.org/gpu/drm-internals.html?highlight=devm_drm_dev_alloc#c.devm_drm_dev_alloc\n> [1]: https://docs.kernel.org/gpu/drm-internals.html?highlight=remove_conflicting_framebuffers#c.drm_aperture_remove_conflicting_framebuffers","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9041CBD160\n\tfor ;\n\tThu, 9 Jun 2022 18:17:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BDED465635;\n\tThu, 9 Jun 2022 20:17:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22E3B6559A\n\tfor ;\n\tThu, 9 Jun 2022 20:17:22 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A29676CF;\n\tThu, 9 Jun 2022 20:17:21 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654798643;\n\tbh=iMHDjO2rgMhWFAylk+mZyPdahyAxenrFkFHEo+ZHr8c=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=iC/ZNUMWrhsw6fpyeLGv+Yxp4sULFuPwc9QQwtzRFyaFqLKZXg1Hpzk6Lcnu0mN5q\n\t5/TCvkOtnEoZkUILjDFWjFAbhY3fWORuxM5wW/wcfe9Xhd1MHfonhPu5GhCn6ZKc+i\n\tB9YfHDEZK8vznhnqku3eIoq+MXEB1uTM0l0NhzHmLNntX0cP41fDRMHhsG1Fjw/pOk\n\tyVcknUZUJ+NFh523rzcPkTrcOvJw5Q3gzZBOnv5dKV/VGw1OdS+z3xs2ktGKb68Ms+\n\tMhe1tijzkaQmlhugJefB3//6bvwR/5RtSmXclctwhXqtDpsgTxiUmAz8gW42W0LGE2\n\tzLGKWmHvpeoeQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654798641;\n\tbh=iMHDjO2rgMhWFAylk+mZyPdahyAxenrFkFHEo+ZHr8c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tXlXUilGhYV12cv1GNidp1ELv3ArVC2Qj1xEajAVHJZLGdp3OWFc4yusC5vzHr6Mn\n\toCAhNdWS/SKQ5JglgsUYXcMSsWCHWbTHjDCu5b29NnbrWoGJNO6SLROCC0TI24XARB\n\tALbF9C2+eX6tb18j/Y1mk81ccmOtZzqf7RH9Z2fE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"tXlXUilG\"; dkim-atps=neutral","Date":"Thu, 9 Jun 2022 21:17:15 +0300","To":"Javier Martinez Canillas ","Message-ID":"","References":"<20220602100017.22272-1-ecurtin@redhat.com>\n\t\n\t\n\t\n\t<610a8ad4-4023-3ed9-b87f-a44bc2aa3bd0@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<610a8ad4-4023-3ed9-b87f-a44bc2aa3bd0@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH v3] cam: drm: Support /dev/dri cards\n\tother than 0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Ian Mullins ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]