[{"id":23478,"web_url":"https://patchwork.libcamera.org/comment/23478/","msgid":"","date":"2022-06-20T08:54:20","subject":"Re: [libcamera-devel] [PATCH v6] 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 Sat, 18 Jun 2022 at 21:06, Laurent Pinchart\n wrote:\n>\n> Hi Eric,\n>\n> Thank you for the patch.\n>\n> On Fri, Jun 17, 2022 at 11:00:44AM +0100, Eric Curtin 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. 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> > Tested-by: Ian Mullins \n> > Signed-off-by: Eric Curtin \n> > ---\n> > Changes in v6:\n> > - Spaces to tabs\n> > - Remove a brace for single line if statement\n> >\n> > Changes in v5:\n> > - Split into openCard function\n> > - ret initialized to -ENOENT, in case directory is open with no card*\n> > - Add another string to simplify\n> >\n> > Changes in v4:\n> > - added Tested-by tag\n> > - std::stringified things\n> > - changed if conditions to reduce indentations\n> > - constexpr sizes now don't include null terminator\n> > - just return -ENOENT if no device was successfully opened.\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 | 59 ++++++++++++++++++++++++++++++++++++-------------\n> > src/cam/drm.h | 2 +-\n> > 2 files changed, 45 insertions(+), 16 deletions(-)\n> >\n> > diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp\n> > index 42c5a3b1..41fd042b 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> > @@ -391,26 +392,54 @@ Device::~Device()\n> > drmClose(fd_);\n> > }\n> >\n> > -int Device::init()\n> > +int Device::openCard()\n> > {\n> > - constexpr size_t NODE_NAME_MAX = sizeof(\"/dev/dri/card255\");\n> > - char name[NODE_NAME_MAX];\n> > - int ret;\n> > + const std::string dirName = \"/dev/dri/\";\n> > + int ret = -ENOENT;\n> >\n> > /*\n> > - * Open the first DRM/KMS device. The libdrm drmOpen*() functions\n> > - * require either a module name or a bus ID, which we don't have, so\n> > - * bypass them. The automatic module loading and device node creation\n> > - * from drmOpen() is of no practical use as any modern system will\n> > - * handle that through udev or an equivalent component.\n> > + * Open the first DRM/KMS device beginning with /dev/dri/card. The\n> > + * libdrm drmOpen*() functions require either a module name or a bus ID,\n> > + * which we don't have, so bypass them. The automatic module loading and\n> > + * device node creation from drmOpen() is of no practical use as any\n> > + * modern system will handle that through udev or an equivalent\n> > + * component.\n> > */\n> > - snprintf(name, sizeof(name), \"/dev/dri/card%u\", 0);\n> > - fd_ = open(name, O_RDWR | O_CLOEXEC);\n> > - if (fd_ < 0) {\n> > + DIR *folder = opendir(dirName.c_str());\n> > + if (!folder) {\n> > ret = -errno;\n> > - std::cerr\n> > - << \"Failed to open DRM/KMS device \" << name << \": \"\n> > - << strerror(-ret) << std::endl;\n> > + std::cerr << \"Failed to open \" << dirName\n> > + << \" directory: \" << strerror(-ret) << std::endl;\n> > + return ret;\n> > + }\n> > +\n> > + for (struct dirent *res; (res = readdir(folder));) {\n> > + if (strncmp(res->d_name, \"card\", 4))\n> > + continue;\n> > +\n> > + const std::string devName = dirName + res->d_name;\n> > + fd_ = open(devName.c_str(), O_RDWR | O_CLOEXEC);\n> > + if (fd_ >= 0) {\n> > + ret = 0;\n> > + break;\n> > + }\n> > +\n> > + ret = -errno;\n> > + std::cerr << \"Failed to open DRM/KMS device \" << devName << \": \"\n> > + << strerror(-ret) << std::endl;\n> > + }\n> > +\n> > + closedir(folder);\n> > +\n> > + return ret;\n> > +}\n> > +\n> > +int Device::init()\n> > +{\n> > + int ret = openCard();\n> > + if (ret < 0) {\n> > + std::cerr << \"Failed to open any DRM/KMS device: \"\n> > + << strerror(-ret) << std::endl;\n> > return ret;\n> > }\n> >\n> > diff --git a/src/cam/drm.h b/src/cam/drm.h\n> > index de57e445..1817d053 100644\n> > --- a/src/cam/drm.h\n> > +++ b/src/cam/drm.h\n> > @@ -312,7 +312,7 @@ private:\n> > Device &operator=(const Device &&) = delete;\n> >\n> > int getResources();\n> > -\n> > + int openCard();\n>\n> Just one small nit, I'd move this before getResources() to match the\n> order in the .cpp file. With that,\n>\n> Reviewed-by: Laurent Pinchart \n>\n> No need to resend, I'll fix this when applying.\n\nOk thanks Laurent.\n\n>\n> > void drmEvent();\n> > static void pageFlipComplete(int fd, unsigned int sequence,\n> > unsigned int tv_sec, unsigned int tv_usec,\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 7A1A4BE173\n\tfor ;\n\tMon, 20 Jun 2022 08:54:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ABABB65635;\n\tMon, 20 Jun 2022 10:54:40 +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 D632860473\n\tfor ;\n\tMon, 20 Jun 2022 10:54:39 +0200 (CEST)","from mail-qv1-f72.google.com (mail-qv1-f72.google.com\n\t[209.85.219.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-286-pyz77LpbNMKXVp2JEkMcXg-1; Mon, 20 Jun 2022 04:54:37 -0400","by mail-qv1-f72.google.com with SMTP id\n\t10-20020a0562140cca00b004702e8ce21bso5550136qvx.22\n\tfor ;\n\tMon, 20 Jun 2022 01:54:37 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655715280;\n\tbh=KYs5LMO4WytufKTrxeJlrcaxaS7tRH1dvE1kSJ9rnfk=;\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=tSyTQetUe7U55NMK1BVHYNM3TKpg5BkRBjRpPIL55Ck1GfxDmDQBD86wD4lB9fWgk\n\t2cU/UodSoHTRmBAOBdna0XKjot3A0UGZ8Jj/9r9Vil+7UgSMZmU0ktps7jIhAouOWW\n\tdaQaswMbuiBLuB5ZtXQFFZCoBlhLbqorNeo+HZxsTu94Ns7LCk6yDqHYb39k43AWVr\n\tnxtsCcBoXqwVXs6lcQorC7He4n+e1rPJygkk7cFG+B9oDUGYC8AApQr0jbelBmx/zX\n\tvPeY93h3nIFBeypBhyFlMqY51e95ASx0Hp2HzHwBC2f3Av5HwQr7w/fqoRsIO+/eQY\n\t6FwJstvv0wZmg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1655715278;\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=PnIeqBsQrh9JTYYg2eS1m6VLwxMoDHz4evjjEK7p218=;\n\tb=i2gGoxgSG+mRN78MUOi5C2Ved14/b0616iZK1dbnc3bBBckkYDdvIY6pl7rNdLOlgNbKbe\n\t5gBSHZevQpsc2ixiKym7R86suutNRd5JRo7zLAfCIpor+Avmg3XAlsWpmkXLzHEDx0TS+J\n\tQYHSZgKbyAEdsGdFD8NI+zSZJxe6QmI="],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=redhat.com\n\theader.i=@redhat.com header.b=\"i2gGoxgS\"; \n\tdkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ecurtin@redhat.com"],"X-MC-Unique":"pyz77LpbNMKXVp2JEkMcXg-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=PnIeqBsQrh9JTYYg2eS1m6VLwxMoDHz4evjjEK7p218=;\n\tb=ZoBA/9zxKOKECbmyk2OxY5qe8EXidwsc2u7N0XRt/K+uOc3Ys3MSR2rrTitHaOTLKJ\n\tUcea6HvCS9LHetBxZNpGLMhQF+nfhnCsgE/vXtcM8WM0b3HAW+iWfr2OfTEluwmbTSQt\n\tjTJdd00okyx+e7dBJVc/pSnhFaymf+8/c6uCvLEYreiPYUYvqCs4vsvtG3lFIgrbfHiW\n\tBx+vw+uIDr0PAqk1RpP3eJG7iok10ZbgFW9+6tokdtEKFlHxybg1Jdugk1TG/dBhDKQb\n\tqiJ7h8Ynxah/VhaIx62jVDxw8rTr/+61FGuA/2nBS0oSo1kAoG39WbdcAAMVtOcZcawT\n\tL7CQ==","X-Gm-Message-State":"AJIora/x8YUuTjOckRHdWRIsH6qXBeOxvczu6zgR6v8TK9y8CsBJzDAu\n\tt3hPcKIkanJMmHFs0/4Vn+T4vb5+lQBL1Qs/rJR98wYpOwwV4GF+z312/dymWP3WTkYMwk628sy\n\twm/GD1H8I4IVWQxWPpNruxku3TBD5QkFFuFFSULQ7vDMo/pvtyw==","X-Received":["by 2002:a05:6214:2a89:b0:464:b97d:b686 with SMTP id\n\tjr9-20020a0562142a8900b00464b97db686mr18107163qvb.65.1655715276605; \n\tMon, 20 Jun 2022 01:54:36 -0700 (PDT)","by 2002:a05:6214:2a89:b0:464:b97d:b686 with SMTP id\n\tjr9-20020a0562142a8900b00464b97db686mr18107154qvb.65.1655715276305;\n\tMon, 20 Jun 2022 01:54:36 -0700 (PDT)"],"X-Google-Smtp-Source":"AGRyM1t2TXN8eV4+vy+jotjUR1iekDVCxe4q/bwmAYxeKi2Bz8Dwee7/u3YGd4OQ/ZFhn9dMb290Yum8bkuuOPXR35U=","MIME-Version":"1.0","References":"<20220617100043.8753-1-ecurtin@redhat.com>\n\t","In-Reply-To":"","Date":"Mon, 20 Jun 2022 09:54:20 +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 v6] 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\" "}}]