[{"id":2318,"web_url":"https://patchwork.libcamera.org/comment/2318/","msgid":"<20190805173439.GD13149@pendragon.ideasonboard.com>","date":"2019-08-05T17:34:39","subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: pipeline: vimc:\n\tPropagate media bus format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Aug 05, 2019 at 05:51:31PM +0200, Niklas Söderlund wrote:\n> Linux commit 85ab1aa1fac17bcd (\"media: vimc: deb: fix default sink bayer\n> format\") which is part of v5.2 changes the default media bus format for\n> the debayer subdevices. This leads to a -EPIPE error when trying to use\n> the raw capture video device nodes.\n> \n> Fix this by propagating the media bus format used on the debayer\n> subdevcie to the sensor. This allows the same code to function on\n> kernels previous to the change and after it.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/pipeline/vimc.cpp | 21 ++++++++++++++++++++-\n> 1 file changed, 20 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 3d6808222a8a2c5d..ae612b48436d1164 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -25,6 +25,7 @@\n> #include \"pipeline_handler.h\"\n> #include \"utils.h\"\n> #include \"v4l2_controls.h\"\n> +#include \"v4l2_subdevice.h\"\n> #include \"v4l2_videodevice.h\"\n> \n> namespace libcamera {\n> @@ -35,13 +36,15 @@ class VimcCameraData : public CameraData\n> {\n> public:\n> \tVimcCameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe), video_(nullptr), sensor_(nullptr)\n> +\t\t: CameraData(pipe), video_(nullptr), debayer_(nullptr),\n> +\t\t sensor_(nullptr)\n> \t{\n> \t}\n> \n> \t~VimcCameraData()\n> \t{\n> \t\tdelete sensor_;\n> +\t\tdelete debayer_;\n> \t\tdelete video_;\n> \t}\n> \n> @@ -49,6 +52,7 @@ public:\n> \tvoid bufferReady(Buffer *buffer);\n> \n> \tV4L2VideoDevice *video_;\n> +\tV4L2Subdevice *debayer_;\n> \tCameraSensor *sensor_;\n> \tStream stream_;\n> };\n> @@ -188,6 +192,17 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \t format.fourcc != cfg.pixelFormat)\n> \t\treturn -EINVAL;\n> \n> +\tV4L2SubdeviceFormat subformat = {};\n> +\tsubformat.size = cfg.size;\n> +\n> +\tret = data->debayer_->setFormat(0, &subformat);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = data->sensor_->setFormat(&subformat);\n> +\tif (ret)\n> +\t\treturn ret;\n\nThe sensor comes before the debayering subdevice, shouldn't you set the\nformat on the sensor first ? That may require setting the bus format in\nsubformat, but I think you can simply hardcode that. I would also\nconfigure this before configuring the video node.\n\n> +\n> \tcfg.setStream(&data->stream_);\n> \n> \treturn 0;\n> @@ -340,6 +355,10 @@ int VimcCameraData::init(MediaDevice *media)\n> \tif (video_->open())\n> \t\treturn -ENODEV;\n> \n> +\tdebayer_ = new V4L2Subdevice(media->getEntityByName(\"Debayer B\"));\n> +\tif (debayer_->open())\n> +\t\treturn -ENODEV;\n> +\n> \tvideo_->bufferReady.connect(this, &VimcCameraData::bufferReady);\n> \n> \tsensor_ = new CameraSensor(media->getEntityByName(\"Sensor B\"));","headers":{"Return-Path":"","Received":["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 3C2B760E33\n\tfor ;\n\tMon, 5 Aug 2019 19:34:42 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AED8A2F9;\n\tMon, 5 Aug 2019 19:34:41 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565026481;\n\tbh=GydAbbiBzjB1oW1dMMdxIT9Zn+CBMuHSMirSQOzWpmU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YhL35Dmh0ojNM2tRCHcjX08QjD76czzJaGxkST5RnTMnWSqsDGmOdwjx9IjqU7QDx\n\tA594uQHiAxyElvVtQNOWcVFblGaerIOsGoKDFqa6FUMuFbLQ+gk3a/QhsOJ+xIhtHz\n\tXrkq2ZtZs7wiQgPF+AooNXhnY3P2TKKmjNi79Qf4=","Date":"Mon, 5 Aug 2019 20:34:39 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190805173439.GD13149@pendragon.ideasonboard.com>","References":"<20190805155133.11335-1-niklas.soderlund@ragnatech.se>\n\t<20190805155133.11335-3-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190805155133.11335-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: pipeline: vimc:\n\tPropagate media bus format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 05 Aug 2019 17:34:42 -0000"}},{"id":2333,"web_url":"https://patchwork.libcamera.org/comment/2333/","msgid":"<20190807202027.GH3186@bigcity.dyn.berto.se>","date":"2019-08-07T20:20:27","subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: pipeline: vimc:\n\tPropagate media bus format","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-08-05 20:34:39 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Mon, Aug 05, 2019 at 05:51:31PM +0200, Niklas Söderlund wrote:\n> > Linux commit 85ab1aa1fac17bcd (\"media: vimc: deb: fix default sink bayer\n> > format\") which is part of v5.2 changes the default media bus format for\n> > the debayer subdevices. This leads to a -EPIPE error when trying to use\n> > the raw capture video device nodes.\n> > \n> > Fix this by propagating the media bus format used on the debayer\n> > subdevcie to the sensor. This allows the same code to function on\n> > kernels previous to the change and after it.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/pipeline/vimc.cpp | 21 ++++++++++++++++++++-\n> > 1 file changed, 20 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index 3d6808222a8a2c5d..ae612b48436d1164 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -25,6 +25,7 @@\n> > #include \"pipeline_handler.h\"\n> > #include \"utils.h\"\n> > #include \"v4l2_controls.h\"\n> > +#include \"v4l2_subdevice.h\"\n> > #include \"v4l2_videodevice.h\"\n> > \n> > namespace libcamera {\n> > @@ -35,13 +36,15 @@ class VimcCameraData : public CameraData\n> > {\n> > public:\n> > \tVimcCameraData(PipelineHandler *pipe)\n> > -\t\t: CameraData(pipe), video_(nullptr), sensor_(nullptr)\n> > +\t\t: CameraData(pipe), video_(nullptr), debayer_(nullptr),\n> > +\t\t sensor_(nullptr)\n> > \t{\n> > \t}\n> > \n> > \t~VimcCameraData()\n> > \t{\n> > \t\tdelete sensor_;\n> > +\t\tdelete debayer_;\n> > \t\tdelete video_;\n> > \t}\n> > \n> > @@ -49,6 +52,7 @@ public:\n> > \tvoid bufferReady(Buffer *buffer);\n> > \n> > \tV4L2VideoDevice *video_;\n> > +\tV4L2Subdevice *debayer_;\n> > \tCameraSensor *sensor_;\n> > \tStream stream_;\n> > };\n> > @@ -188,6 +192,17 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> > \t format.fourcc != cfg.pixelFormat)\n> > \t\treturn -EINVAL;\n> > \n> > +\tV4L2SubdeviceFormat subformat = {};\n> > +\tsubformat.size = cfg.size;\n> > +\n> > +\tret = data->debayer_->setFormat(0, &subformat);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tret = data->sensor_->setFormat(&subformat);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> \n> The sensor comes before the debayering subdevice, shouldn't you set the\n> format on the sensor first ? That may require setting the bus format in\n> subformat, but I think you can simply hardcode that. I would also\n> configure this before configuring the video node.\n\nI tried that at first but there seems more work is required on the vimc \nkernel driver upstream and this was a way to mitigate the current state. \nI agree it was not the best idea.\n\nAfter closer looking at the problem it seems that the only way to \nsupport v5.2 and v5.1 (and earlier) in a nicer way is to turn the raw \ncapture camera into only dealing with bayer formats. This don't seem to \nbe the intent of the Linux driver as it exposes other formats but they \nall render -EPIPE on either v5.1 or v5.2. I have seen some Revert \ncommits on the linux-media ML but they seems to have died out in \nactivity [1] :-(\n\nI'm not sure what is the best way forward here, turning vimc into only \ndelivering bayer formats is not ideal but I fear it's our best option \nuntil upstream is fixed. I have patches for this and I will send a v2 of \nthis series doing this, but I'm open to other solutions which will \nunblock running on v5.2 (and v5.1) as that is my development environment \nwaiting for upstream is not an appetizing option for me.\n\n1. https://lkml.org/lkml/2019/7/9/682\n\n> \n> > +\n> > \tcfg.setStream(&data->stream_);\n> > \n> > \treturn 0;\n> > @@ -340,6 +355,10 @@ int VimcCameraData::init(MediaDevice *media)\n> > \tif (video_->open())\n> > \t\treturn -ENODEV;\n> > \n> > +\tdebayer_ = new V4L2Subdevice(media->getEntityByName(\"Debayer B\"));\n> > +\tif (debayer_->open())\n> > +\t\treturn -ENODEV;\n> > +\n> > \tvideo_->bufferReady.connect(this, &VimcCameraData::bufferReady);\n> > \n> > \tsensor_ = new CameraSensor(media->getEntityByName(\"Sensor B\"));\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE7D560E31\n\tfor ;\n\tWed, 7 Aug 2019 22:20:29 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id a30so1937758lfk.12\n\tfor ;\n\tWed, 07 Aug 2019 13:20:29 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tq6sm18619938lji.70.2019.08.07.13.20.28\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tWed, 07 Aug 2019 13:20:28 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=Bi1w5aMop6DGSl56t106+8vVx0e1XKhkZBnF66IB5G4=;\n\tb=1MIS4JkNUCESlTqKPnzQGgQ5HkZLIRDKK6oJck6xwiQNbXc0nFy9GZiAmax7qG481U\n\t7QkOpHTK1fqNFbGPqA/bU3Sjc0+oRRKi5kpgbumOId2/Cw5BzC+2OKyA4yNE9tpjQ0+l\n\tNhy53G2mVKaWl7luIl/IDLUMxxU4NtJ9S1mt0pX/dyPMosm4AmtBG4SLlpBMXCGJr2Kg\n\tckye84TrmwHZFvN5xjaztcedH6SUVYAiFB11T12FSbFKak7F3dPJXON5+boeEgcPfraT\n\tWEOEGS9lL5TSFNt5DOy9AUhiXfBoDHoPFxhC2SH+Io/8sVVYQnW4f7ap72ciHg+9TgOd\n\tR8NA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=Bi1w5aMop6DGSl56t106+8vVx0e1XKhkZBnF66IB5G4=;\n\tb=hKcPkbBf4nfjhdxPqW/A0Dj8BvuGBdjG2oaDkvYIDdhNU7wmMFjMrZl0KbUpuxn/65\n\tM2njyVdUjZcy3BqbOCwDNe/cgy9HEuWdev8AbcD2eOGUhWHCssNh/oLGolk9mc3Lhn7S\n\t8TykD8RCyTEYApv7yDn3msg+PTsChfDt3EGF+U4b2Lf7V3r/Jt3LDwFFJyWGU5IJYn1a\n\tdA+2QUdxo5BQoMFd8KzGXOl8fvUKkZ4xoeWsGpNwoFhEdnvnvTBBppXxVqfWLiFJnN4E\n\ttbedQrqOyeG1zOhmX0qlnJFKw4sVcTxH5hFZpksS3Iz492HRiTwUfDI4/uPXEO/SLg4f\n\tcdKg==","X-Gm-Message-State":"APjAAAU52HEfyBx7a7vrgs/hJewUKXz85iX0UwTVjYWmG1kbDcC0wUeK\n\tDVM0tjWxp2i5br0wwUu4czGq/GEizJM=","X-Google-Smtp-Source":"APXvYqy+nnh75FVCPLeeoiQA+CsO+rUUWDuZ+OSmhII79FAn/JTdohceoqt8jMl+H707TrkkhZly4A==","X-Received":"by 2002:ac2:5c42:: with SMTP id s2mr7120375lfp.61.1565209228918; \n\tWed, 07 Aug 2019 13:20:28 -0700 (PDT)","Date":"Wed, 7 Aug 2019 22:20:27 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190807202027.GH3186@bigcity.dyn.berto.se>","References":"<20190805155133.11335-1-niklas.soderlund@ragnatech.se>\n\t<20190805155133.11335-3-niklas.soderlund@ragnatech.se>\n\t<20190805173439.GD13149@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190805173439.GD13149@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 2/4] libcamera: pipeline: vimc:\n\tPropagate media bus format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 07 Aug 2019 20:20:30 -0000"}}]