[{"id":22129,"web_url":"https://patchwork.libcamera.org/comment/22129/","msgid":"<YgGdAJfMxeIZ0TYl@pendragon.ideasonboard.com>","date":"2022-02-07T22:28:16","subject":"Re: [libcamera-devel] [PATCH v3] cam: kms-sink: Once we have found\n\ta suitable pipeline to select, return","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 Mon, Feb 07, 2022 at 03:01:36PM +0000, Eric Curtin wrote:\n> The break only breaks out of the innermost loop which is not the most\n> optimal. It also breaks cam on my machines.\n\nI'd expand this a bit.\n\ncam: kms_sink: Use the first suitable pipeline found\n\nWhen searching for a suitable pipeline, we mistakenly only break from\nthe inner loop. This results in the last suitable output being selected.\nPick the first one instead.\n\n> Fixes: 1de0f90dd432 (\"cam: kms_sink: Print display pipelineconfiguration\")\n> Signed-off-by: Eric Curtin <ecurtin@redhat.com>\n> ---\n> \n> Changes in v3:\n> - Add \"cam: kms_sink:\" tag\n> \n> Changes in v2:\n> - Change function name to selectPipeline\n> - Return int rather than pointer\n> - Formatting\n> - Mention in commit message that it fixes a bug on my machine also\n> \n>  src/cam/kms_sink.cpp | 18 ++++++++++++------\n>  src/cam/kms_sink.h   |  1 +\n>  2 files changed, 13 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp\n> index d30fba78..973cd370 100644\n> --- a/src/cam/kms_sink.cpp\n> +++ b/src/cam/kms_sink.cpp\n> @@ -136,7 +136,7 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n>  \treturn 0;\n>  }\n>  \n> -int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> +int KMSSink::selectPipeline(const libcamera::PixelFormat &format)\n>  {\n>  \t/*\n>  \t * If the requested format has an alpha channel, also consider the X\n> @@ -174,25 +174,31 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n>  \t\t\t\t\tcrtc_ = crtc;\n>  \t\t\t\t\tplane_ = plane;\n>  \t\t\t\t\tformat_ = format;\n> -\t\t\t\t\tbreak;\n> +\t\t\t\t\treturn 0;\n>  \t\t\t\t}\n>  \n>  \t\t\t\tif (plane->supportsFormat(xFormat)) {\n>  \t\t\t\t\tcrtc_ = crtc;\n>  \t\t\t\t\tplane_ = plane;\n>  \t\t\t\t\tformat_ = xFormat;\n> -\t\t\t\t\tbreak;\n> +\t\t\t\t\treturn 0;\n>  \t\t\t\t}\n>  \t\t\t}\n>  \t\t}\n>  \t}\n>  \n> -\tif (!crtc_) {\n> +\treturn -EPIPE;\n> +}\n> +\n> +int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> +{\n> +\tconst int ret = selectPipeline(format);\n> +\tif (ret) {\n>  \t\tstd::cerr\n>  \t\t\t<< \"Unable to find display pipeline for format \"\n>  \t\t\t<< format.toString() << std::endl;\n>  \n> -\t\treturn -EPIPE;\n> +\t\treturn ret;\n>  \t}\n>  \n>  \tstd::cout\n> @@ -200,7 +206,7 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n>  \t\t<< \", connector \" << connector_->name()\n>  \t\t<< \" (\" << connector_->id() << \")\" << std::endl;\n>  \n> -\treturn 0;\n> +\treturn ret;\n\nThis change isn't needed.\n\nWith these changes,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nIf you agree with those changes there's no need to resubmit, I'll make\nthose small modifications before pushing.\n\n>  }\n>  \n>  int KMSSink::start()\n> diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h\n> index 1e4290ad..4a0a872c 100644\n> --- a/src/cam/kms_sink.h\n> +++ b/src/cam/kms_sink.h\n> @@ -47,6 +47,7 @@ private:\n>  \t\tlibcamera::Request *camRequest_;\n>  \t};\n>  \n> +\tint selectPipeline(const libcamera::PixelFormat &format);\n>  \tint configurePipeline(const libcamera::PixelFormat &format);\n>  \tvoid requestComplete(DRM::AtomicRequest *request);\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 34958BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  7 Feb 2022 22:28:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69FA261086;\n\tMon,  7 Feb 2022 23:28:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 009A0609B9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  7 Feb 2022 23:28:18 +0100 (CET)","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 5A38B499;\n\tMon,  7 Feb 2022 23:28:18 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QlKWyT7A\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1644272898;\n\tbh=MtA64o8mR6xFy5LvVDE75+OiuBFYjQF+5x0TIAg2r+o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QlKWyT7At5WhbGVQqXKfXloan9JhhFa4sFrr7B3WUbk00ZlbxH7S10GruuaHsfOrP\n\tBSaHb1Rzq8+o+NQrz7qm5Hhc+5lcIOkwmtgFBuAKfjVACYewaR1eBFgILFX0WfjJUL\n\t66a1zeKT7y1mbbXC3fILe5gQtnT8SmwAF1R7Lj/Y=","Date":"Tue, 8 Feb 2022 00:28:16 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Eric Curtin <ecurtin@redhat.com>","Message-ID":"<YgGdAJfMxeIZ0TYl@pendragon.ideasonboard.com>","References":"<20220207150136.22584-1-ecurtin@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220207150136.22584-1-ecurtin@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH v3] cam: kms-sink: Once we have found\n\ta suitable pipeline to select, return","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22136,"web_url":"https://patchwork.libcamera.org/comment/22136/","msgid":"<CAOgh=FxexLzqTPaV2jR9akga_Pw_0rjYdP0yQt0aGTfCQmz-=A@mail.gmail.com>","date":"2022-02-08T08:09:57","subject":"Re: [libcamera-devel] [PATCH v3] cam: kms-sink: Once we have found\n\ta suitable pipeline to select, return","submitter":{"id":101,"url":"https://patchwork.libcamera.org/api/people/101/","name":"Eric Curtin","email":"ecurtin@redhat.com"},"content":"On Mon, 7 Feb 2022 at 22:28, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Eric,\n>\n> Thank you for the patch.\n>\n> On Mon, Feb 07, 2022 at 03:01:36PM +0000, Eric Curtin wrote:\n> > The break only breaks out of the innermost loop which is not the most\n> > optimal. It also breaks cam on my machines.\n>\n> I'd expand this a bit.\n>\n> cam: kms_sink: Use the first suitable pipeline found\n>\n> When searching for a suitable pipeline, we mistakenly only break from\n> the inner loop. This results in the last suitable output being selected.\n> Pick the first one instead.\n>\n> > Fixes: 1de0f90dd432 (\"cam: kms_sink: Print display pipelineconfiguration\")\n> > Signed-off-by: Eric Curtin <ecurtin@redhat.com>\n> > ---\n> >\n> > Changes in v3:\n> > - Add \"cam: kms_sink:\" tag\n> >\n> > Changes in v2:\n> > - Change function name to selectPipeline\n> > - Return int rather than pointer\n> > - Formatting\n> > - Mention in commit message that it fixes a bug on my machine also\n> >\n> >  src/cam/kms_sink.cpp | 18 ++++++++++++------\n> >  src/cam/kms_sink.h   |  1 +\n> >  2 files changed, 13 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp\n> > index d30fba78..973cd370 100644\n> > --- a/src/cam/kms_sink.cpp\n> > +++ b/src/cam/kms_sink.cpp\n> > @@ -136,7 +136,7 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n> >       return 0;\n> >  }\n> >\n> > -int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> > +int KMSSink::selectPipeline(const libcamera::PixelFormat &format)\n> >  {\n> >       /*\n> >        * If the requested format has an alpha channel, also consider the X\n> > @@ -174,25 +174,31 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> >                                       crtc_ = crtc;\n> >                                       plane_ = plane;\n> >                                       format_ = format;\n> > -                                     break;\n> > +                                     return 0;\n> >                               }\n> >\n> >                               if (plane->supportsFormat(xFormat)) {\n> >                                       crtc_ = crtc;\n> >                                       plane_ = plane;\n> >                                       format_ = xFormat;\n> > -                                     break;\n> > +                                     return 0;\n> >                               }\n> >                       }\n> >               }\n> >       }\n> >\n> > -     if (!crtc_) {\n> > +     return -EPIPE;\n> > +}\n> > +\n> > +int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> > +{\n> > +     const int ret = selectPipeline(format);\n> > +     if (ret) {\n> >               std::cerr\n> >                       << \"Unable to find display pipeline for format \"\n> >                       << format.toString() << std::endl;\n> >\n> > -             return -EPIPE;\n> > +             return ret;\n> >       }\n> >\n> >       std::cout\n> > @@ -200,7 +206,7 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> >               << \", connector \" << connector_->name()\n> >               << \" (\" << connector_->id() << \")\" << std::endl;\n> >\n> > -     return 0;\n> > +     return ret;\n>\n> This change isn't needed.\n>\n> With these changes,\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n> If you agree with those changes there's no need to resubmit, I'll make\n> those small modifications before pushing.\n\nMeant to reply all, sounds good to me.\n\n>\n> >  }\n> >\n> >  int KMSSink::start()\n> > diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h\n> > index 1e4290ad..4a0a872c 100644\n> > --- a/src/cam/kms_sink.h\n> > +++ b/src/cam/kms_sink.h\n> > @@ -47,6 +47,7 @@ private:\n> >               libcamera::Request *camRequest_;\n> >       };\n> >\n> > +     int selectPipeline(const libcamera::PixelFormat &format);\n> >       int configurePipeline(const libcamera::PixelFormat &format);\n> >       void requestComplete(DRM::AtomicRequest *request);\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A1848BDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  8 Feb 2022 08:10:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CDE26610A2;\n\tTue,  8 Feb 2022 09:10:18 +0100 (CET)","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 DD68460E55\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Feb 2022 09:10:16 +0100 (CET)","from mail-oo1-f72.google.com (mail-oo1-f72.google.com\n\t[209.85.161.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-524--Jsyhz2oMtaFO5JyJ41OoA-1; Tue, 08 Feb 2022 03:10:14 -0500","by mail-oo1-f72.google.com with SMTP id\n\tv23-20020a4ae057000000b003178bf9f508so4232980oos.14\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 08 Feb 2022 00:10:14 -0800 (PST)"],"Authentication-Results":["lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"DMmgzYX5\"; dkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ecurtin@redhat.com"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1644307815;\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=yXGVfkaGvo901zDfQC5uhCK9FFwyQD4/ricIujgS5IQ=;\n\tb=DMmgzYX5oN9XjJhKLXoAU0m3PdAL25JAFHge5DjOk9zTJ+dW2CkF01kRZW9Q+3RIonoj/6\n\tkUeufzhqW95/oGk22ilgToyiYkeebhNDnXfm634DHVsiFHfuMzO3BHaTYyw1j4ubJC+PWw\n\tUR565oZcA0qT+4YOTl34pmliFUaa/9Y=","X-MC-Unique":"-Jsyhz2oMtaFO5JyJ41OoA-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=yXGVfkaGvo901zDfQC5uhCK9FFwyQD4/ricIujgS5IQ=;\n\tb=6nUhuWOjthyYo+N1ssFVIaL1HsIYmKhCxDkTdLxZS7wceR/+rfy2m6dTlpejyWrfcF\n\t/cIB+GuoqiUfxB53igXyFL4tu57gsfsjrm8ozE8rXzghOvbQdyIVIWaSEFDZyH57ULGH\n\t44M69hLFuBLGFpANNY+fq4gHkJRUBX7ocV5NervWQRQTXRFhSFQYUqxvlC+XgjIGfnUF\n\tTfu60B30GT4txngM1ecAj6LFd1IlhLta8RPolY1icooaGf7yAa30ZfXHbZqf1uvkqlWE\n\tRJVR8sQlqikoXqXJDX5o6FWVuTPpHcdoAX/AGjg2UlSWKeTC7iHllvhwRtbgS6UjwadV\n\t15PA==","X-Gm-Message-State":"AOAM5334iupH2tT8gDnDBhl5oG4gy76lGiVNmrZgrI/en5Iu5Hh8DP9N\n\tsla34bAp6yJH2WXOFSeinIJisVvYKgoVQB57IM/N1VnhQ3raKAsP6nS9J/00OdU5hc/l+n3WwmQ\n\tOcN3MpTj0tscS3KLdqdIAJXXe3SFiu5plVQpyW7GZsqMW5c2MjA==","X-Received":["by 2002:a54:4486:: with SMTP id v6mr13902oiv.121.1644307813447; \n\tTue, 08 Feb 2022 00:10:13 -0800 (PST)","by 2002:a54:4486:: with SMTP id v6mr13898oiv.121.1644307813122; \n\tTue, 08 Feb 2022 00:10:13 -0800 (PST)"],"X-Google-Smtp-Source":"ABdhPJxsT/XsAO1Db1cqxOCiP/GO4vk0dYiLTf+sKZVw73tYzPS7w+8kvStQ/OmDvtifEgiuPZfP77dgKBqnxCTz4NY=","MIME-Version":"1.0","References":"<20220207150136.22584-1-ecurtin@redhat.com>\n\t<YgGdAJfMxeIZ0TYl@pendragon.ideasonboard.com>","In-Reply-To":"<YgGdAJfMxeIZ0TYl@pendragon.ideasonboard.com>","From":"Eric Curtin <ecurtin@redhat.com>","Date":"Tue, 8 Feb 2022 08:09:57 +0000","Message-ID":"<CAOgh=FxexLzqTPaV2jR9akga_Pw_0rjYdP0yQt0aGTfCQmz-=A@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v3] cam: kms-sink: Once we have found\n\ta suitable pipeline to select, return","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22141,"web_url":"https://patchwork.libcamera.org/comment/22141/","msgid":"<164431279565.2908941.7070625461006288612@Monstersaurus>","date":"2022-02-08T09:33:15","subject":"Re: [libcamera-devel] [PATCH v3] cam: kms-sink: Once we have found\n\ta suitable pipeline to select, return","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-02-07 22:28:16)\n> Hi Eric,\n> \n> Thank you for the patch.\n> \n> On Mon, Feb 07, 2022 at 03:01:36PM +0000, Eric Curtin wrote:\n> > The break only breaks out of the innermost loop which is not the most\n> > optimal. It also breaks cam on my machines.\n> \n> I'd expand this a bit.\n> \n> cam: kms_sink: Use the first suitable pipeline found\n> \n> When searching for a suitable pipeline, we mistakenly only break from\n> the inner loop. This results in the last suitable output being selected.\n> Pick the first one instead.\n> \n> > Fixes: 1de0f90dd432 (\"cam: kms_sink: Print display pipelineconfiguration\")\n> > Signed-off-by: Eric Curtin <ecurtin@redhat.com>\n> > ---\n> > \n> > Changes in v3:\n> > - Add \"cam: kms_sink:\" tag\n> > \n> > Changes in v2:\n> > - Change function name to selectPipeline\n> > - Return int rather than pointer\n> > - Formatting\n> > - Mention in commit message that it fixes a bug on my machine also\n> > \n> >  src/cam/kms_sink.cpp | 18 ++++++++++++------\n> >  src/cam/kms_sink.h   |  1 +\n> >  2 files changed, 13 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp\n> > index d30fba78..973cd370 100644\n> > --- a/src/cam/kms_sink.cpp\n> > +++ b/src/cam/kms_sink.cpp\n> > @@ -136,7 +136,7 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)\n> >       return 0;\n> >  }\n> >  \n> > -int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> > +int KMSSink::selectPipeline(const libcamera::PixelFormat &format)\n> >  {\n> >       /*\n> >        * If the requested format has an alpha channel, also consider the X\n> > @@ -174,25 +174,31 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> >                                       crtc_ = crtc;\n> >                                       plane_ = plane;\n> >                                       format_ = format;\n> > -                                     break;\n> > +                                     return 0;\n> >                               }\n> >  \n> >                               if (plane->supportsFormat(xFormat)) {\n> >                                       crtc_ = crtc;\n> >                                       plane_ = plane;\n> >                                       format_ = xFormat;\n> > -                                     break;\n> > +                                     return 0;\n> >                               }\n> >                       }\n> >               }\n> >       }\n> >  \n> > -     if (!crtc_) {\n> > +     return -EPIPE;\n> > +}\n> > +\n> > +int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> > +{\n> > +     const int ret = selectPipeline(format);\n> > +     if (ret) {\n> >               std::cerr\n> >                       << \"Unable to find display pipeline for format \"\n> >                       << format.toString() << std::endl;\n> >  \n> > -             return -EPIPE;\n> > +             return ret;\n> >       }\n> >  \n> >       std::cout\n> > @@ -200,7 +206,7 @@ int KMSSink::configurePipeline(const libcamera::PixelFormat &format)\n> >               << \", connector \" << connector_->name()\n> >               << \" (\" << connector_->id() << \")\" << std::endl;\n> >  \n> > -     return 0;\n> > +     return ret;\n> \n\nIn fact that was the only part that stopped me sending a tag yesterday\nas I didn't spend the time to go look up if this return ret made sense.\n\n> This change isn't needed.\n> \n> With these changes,\n> \n\nWith that, \n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> If you agree with those changes there's no need to resubmit, I'll make\n> those small modifications before pushing.\n> \n> >  }\n> >  \n> >  int KMSSink::start()\n> > diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h\n> > index 1e4290ad..4a0a872c 100644\n> > --- a/src/cam/kms_sink.h\n> > +++ b/src/cam/kms_sink.h\n> > @@ -47,6 +47,7 @@ private:\n> >               libcamera::Request *camRequest_;\n> >       };\n> >  \n> > +     int selectPipeline(const libcamera::PixelFormat &format);\n> >       int configurePipeline(const libcamera::PixelFormat &format);\n> >       void requestComplete(DRM::AtomicRequest *request);\n> >  \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AB84ABDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  8 Feb 2022 09:33:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 151EA610B3;\n\tTue,  8 Feb 2022 10:33:20 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B00DB610AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Feb 2022 10:33:18 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 45FB897;\n\tTue,  8 Feb 2022 10:33:18 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TqktLz4w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1644312798;\n\tbh=lIycfz+YoVSG6hnrdKI+LeRaE28tcScaj7F+wZvsOUM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=TqktLz4wvPxggxiflGTG+bUfSmEXYAlwbvb6q65AgSkmVZ0MyBtE3SIGYEhmEiJ6z\n\tV+UZqp5AmCIFkRhTpTW6GpfnvQP+TCcx9THwxoauCyiS90MVeXxitFHbvbfQX/TtmH\n\tewfkkIfAmC3Tz2Ogh5RIyswv2SNEH5f+f1NIlfOY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<YgGdAJfMxeIZ0TYl@pendragon.ideasonboard.com>","References":"<20220207150136.22584-1-ecurtin@redhat.com>\n\t<YgGdAJfMxeIZ0TYl@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Eric Curtin <ecurtin@redhat.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Tue, 08 Feb 2022 09:33:15 +0000","Message-ID":"<164431279565.2908941.7070625461006288612@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3] cam: kms-sink: Once we have found\n\ta suitable pipeline to select, return","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]