[{"id":3919,"web_url":"https://patchwork.libcamera.org/comment/3919/","msgid":"<20200303102627.2b726d83@eldfell.localdomain>","date":"2020-03-03T08:26:27","subject":"Re: [libcamera-devel] [PATCH] drm/fourcc: Add bayer formats and\n\tmodifiers","submitter":{"id":35,"url":"https://patchwork.libcamera.org/api/people/35/","name":"Pekka Paalanen","email":"ppaalanen@gmail.com"},"content":"On Fri, 28 Feb 2020 17:31:35 +0100\nNiklas Söderlund <niklas.soderlund@ragnatech.se> wrote:\n\n> Bayer formats are used with cameras and contain green, red and blue\n> components, with alternating lines of red and green, and blue and green\n> pixels in different orders. For each block of 2x2 pixels there is one\n> pixel with a red filter, two with a green filter, and one with a blue\n> filter. The filters can be arranged in different patterns.\n> \n> Add DRM fourcc formats to describe the most common Bayer formats. Also\n> add a modifiers to describe the custom packing layouts used by the Intel\n> IPU3 and in the MIPI (Mobile Industry Processor Interface) CSI-2\n> specification.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/uapi/drm/drm_fourcc.h | 95 +++++++++++++++++++++++++++++++++++\n>  1 file changed, 95 insertions(+)\n\nHi,\n\nhere are some by-stander comments.\n\n> \n> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h\n> index 8bc0b31597d80737..561d5a08ffd16b69 100644\n> --- a/include/uapi/drm/drm_fourcc.h\n> +++ b/include/uapi/drm/drm_fourcc.h\n> @@ -286,6 +286,62 @@ extern \"C\" {\n>  #define DRM_FORMAT_YVU444\tfourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */\n>  \n>  \n> +/*\n> + * Bayer formats\n> + *\n> + * Bayer formats contain green, red and blue components, with alternating lines\n> + * of red and green, and blue and green pixels in different orders. For each\n> + * block of 2x2 pixels there is one pixel with a red filter, two with a green\n> + * filter, and one with a blue filter. The filters can be arranged in different\n> + * patterns.\n> + *\n> + * For example, RGGB:\n> + *\trow0: RGRGRGRG...\n> + *\trow1: GBGBGBGB...\n> + *\trow3: RGRGRGRG...\n> + *\trow4: GBGBGBGB...\n> + *\t...\n> + *\n> + * Vendors have different methods to pack the sampling formats to increase data\n> + * density. For this reason the fourcc only describes pixel sample size and the\n> + * filter pattern for each block of 2x2 pixels. A modifier is needed to\n> + * describe the memory layout.\n> + *\n> + * In addition to vendor modifiers for memory layout DRM_FORMAT_MOD_LINEAR may\n> + * be used to describe a layout where all samples are placed consecutively in\n> + * memory. If the sample does not fit inside a single byte, the sample storage\n> + * is extended to the minimum number of (little endian) bytes that can hold the\n> + * sample and any unused most-significant bits are defined as padding.\n\n\"Minimum number of (little endian) bytes\" is probably not quite right,\nbecause you could end up with a 3-byte word for e.g. 18-bit samples,\nand for those I don't think endianess is even a defined concept. Yes,\nyou don't add any >16 bit formats here, but being careful here avoids\nhaving to face the question and confusion when someone does add such\nformats.\n\nAlternatively, do not even pretend to define any layout for samples >\n16 bits, and leave it for the future to be defined if/when the need\narises.\n\n> + *\n> + * For example, SRGGB10:\n> + * Each 10-bit sample is contained in 2 consecutive little endian bytes, where\n> + * the 6 most-significant bits are unused.\n\nNitpick: I think you mean \"10-bit sample is contained in a uint16 word\n(little endian), ...\"\n\n\"little endian byte\" sounds like a strange concept to me, as it seems\nto delve into the order of bits in a byte, MSB or LSB first. I suspect\nmost people would not even think of this, but I've been scarred by\nreading the Pixman pixel format definitions.\n\n> + */\n> +\n> +/* 8-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB8\tfourcc_code('R', 'G', 'G', 'B')\n\nThe S in SRGGB is quite surprising to me. I saw it mentioned in IRC\nthat it is easy to read as sRGB and I agree. I would not know to\nassociate S with Bayer to begin with.\n\nWhy not e.g. DRM_FORMAT_BAYER_RGGB8?\n\n> +#define DRM_FORMAT_SGRBG8\tfourcc_code('G', 'R', 'B', 'G')\n> +#define DRM_FORMAT_SGBRG8\tfourcc_code('G', 'B', 'R', 'G')\n> +#define DRM_FORMAT_SBGGR8\tfourcc_code('B', 'A', '8', '1')\n> +\n> +/* 10-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB10\tfourcc_code('R', 'G', '1', '0')\n> +#define DRM_FORMAT_SGRBG10\tfourcc_code('B', 'A', '1', '0')\n> +#define DRM_FORMAT_SGBRG10\tfourcc_code('G', 'B', '1', '0')\n> +#define DRM_FORMAT_SBGGR10\tfourcc_code('B', 'G', '1', '0')\n> +\n> +/* 12-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB12\tfourcc_code('R', 'G', '1', '2')\n> +#define DRM_FORMAT_SGRBG12\tfourcc_code('B', 'A', '1', '2')\n\nConflict:\n\n#define DRM_FORMAT_BGRA4444     fourcc_code('B', 'A', '1', '2') /* [15:0] B:G:R:A 4:4:4:4 little endian */\n\nDoes the kernel not have a \"self-test\" that ensures that all format\ncodes (and why not modifiers as well) are unique?\n\n\n> +#define DRM_FORMAT_SGBRG12\tfourcc_code('G', 'B', '1', '2')\n> +#define DRM_FORMAT_SBGGR12\tfourcc_code('B', 'G', '1', '2')\n> +\n> +/* 14-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB14\tfourcc_code('R', 'G', '1', '4')\n> +#define DRM_FORMAT_SGRBG14\tfourcc_code('B', 'A', '1', '4')\n> +#define DRM_FORMAT_SGBRG14\tfourcc_code('G', 'B', '1', '4')\n> +#define DRM_FORMAT_SBGGR14\tfourcc_code('B', 'G', '1', '4')\n> +\n>  /*\n>   * Format Modifiers:\n>   *\n> @@ -309,6 +365,7 @@ extern \"C\" {\n>  #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07\n>  #define DRM_FORMAT_MOD_VENDOR_ARM     0x08\n>  #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> +#define DRM_FORMAT_MOD_VENDOR_MIPI 0x0a\n>  \n>  /* add more to the end as needed */\n>  \n> @@ -434,6 +491,17 @@ extern \"C\" {\n>   */\n>  #define I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS fourcc_mod_code(INTEL, 7)\n>  \n> +\n> +/*\n> + * IPU3 Bayer packing layout\n> + *\n> + * The IPU3 raw Bayer formats use a custom packing layout where there are no\n> + * gaps between each 10-bit sample. It packs 25 pixels into 32 bytes leaving\n> + * the 6 most significant bits in the last byte unused. The format is little\n> + * endian.\n\nDo I understand that right, that the collection of bytes (not words?)\nrepresents a stream of bits? In which order do you read the bits of a byte\nto produce the bits in a 10-bit unit?\n\nI don't think \"little endian\" specifies that (even less for\nnon-2/4/8-byte units), and Pixman formats prove that the order could be\nspecified either way.\n\nDoes the \"little endian\" mean that in the 32 bytes long unit, one needs\nto extract in chunks of uint16_t/uint32_t/uint64_t and inspect the bits\nof those words, or is the 32-byte unit supposed to be read byte by byte\nin which case endianess plays no role?\n\nIf possible, it would be good to reword the definition so that these\nquestions cannot arise.\n\n> + */\n> +#define IPU3_FORMAT_MOD_PACKED fourcc_mod_code(INTEL, 8)\n> +\n>  /*\n>   * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks\n>   *\n> @@ -804,6 +872,33 @@ extern \"C\" {\n>   */\n>  #define DRM_FORMAT_MOD_ALLWINNER_TILED fourcc_mod_code(ALLWINNER, 1)\n>  \n> +/* Mobile Industry Processor Interface (MIPI) modifiers */\n> +\n> +/*\n> + * MIPI CSI-2 packing layout\n> + *\n> + * The CSI-2 RAW formats (for example Bayer) use a different packing layout\n> + * depenindg on the sample size.\n> + *\n> + * - 10-bits per sample\n> + *   Every four consecutive samples are packed into 5 bytes. Each of the first 4\n> + *   bytes contain the 8 high order bits of the pixels, and the 5th byte\n> + *   contains the 2 least-significant bits of each pixel, in the same order.\n\n...in the same order? So bits 0-1 are the bits 0-1 of the 1st sample, bits\n2-3 are the bits 0-1 of the 2nd sample, etc?\n\n> + *\n> + * - 12-bits per sample\n> + *   Every two consecutive samples are packed into three bytes. Each of the\n> + *   first two bytes contain the 8 high order bits of the pixels, and the third\n> + *   byte contains the four least-significant bits of each pixel, in the same\n> + *   order.\n> + *\n> + * - 14-bits per sample\n> + *   Every four consecutive samples are packed into seven bytes. Each of the\n> + *   first four bytes contain the eight high order bits of the pixels, and the\n> + *   three following bytes contains the six least-significant bits of each\n> + *   pixel, in the same order.\n\nHow do you count the bits when crossing the byte boundary on the last 3\nbytes? Is it an imaginary 24-bit word, or do you take it byte by byte\nlike this:\n\nbyte0 bits 0-5: 1st sample bits 0-5\nbyte0 bits 6-7: 2nd sample bits 0-1\nbyte1 bits 0-3: 2nd sample bits 2-5\nbyte1 bits 4-7: 3rd sample bits 0-3\nbyte2 bits 0-1: 3rd sample bits 4-5\nbyte2 bits 2-7: 4th sample bits 0-5\n\n> + */\n> +#define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n> +\n>  #if defined(__cplusplus)\n>  }\n>  #endif\n\n\nThanks,\npq","headers":{"Return-Path":"<ppaalanen@gmail.com>","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A13AA60426\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Mar 2020 09:26:40 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id q23so2535826ljm.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 03 Mar 2020 00:26:40 -0800 (PST)","from eldfell.localdomain ([194.136.85.206])\n\tby smtp.gmail.com with ESMTPSA id\n\tx18sm11637099lfc.42.2020.03.03.00.26.38\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 03 Mar 2020 00:26:38 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:in-reply-to:references\n\t:mime-version; bh=+QU6RXQo1P/2OCmr+2NuwBvY/LL5dWND9aTgayHtBpA=;\n\tb=GAZq8MHnfsqgONvwWSbxIh4/uHHYCS9uN5OTN0UmwKKyFDKTZp4MKQ4cgmCAMUOEL7\n\t2O5ntIen0OCJUhQE68qkVUmqZm9Sm04zj+aT9UH3xfS6KX5HCivUJksyuKj0AXWjuDgw\n\t08FiJcqQ+ozmoXuodyxQPCowDC1M/2Hr9OCBSm+v8exslW4evvOknj6752K4TrXynkAk\n\ttqAI7tyLdvRmw4OLrcOLRbzcrRylhFroRf+kn4/FTrCpCc60QUw9BMQsFVGxeVU3KpeC\n\tge+YHtjVDONE+XeNX1ewbpO7EHFELRoti7W64MiA/oCltFZRY82sJ2IkyBBTfEaArZOi\n\tfS/g==","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:in-reply-to\n\t:references:mime-version;\n\tbh=+QU6RXQo1P/2OCmr+2NuwBvY/LL5dWND9aTgayHtBpA=;\n\tb=nlkyM8nMTKbdKeRs3pt3vb9Tq0pJe3RbVvmHh2OGOHxCVhI7ZlSMk2kMRpcLVkcFD/\n\tplg00LXTrNinu2jHIGrFp8IJiwvoeTDek4ZXDllr1/qihIWcY9AzPzDqctdull/kdRL8\n\tI+VAnnQ1kJmjSTcwi3ycYtUOcxgWS/PQKQQ6esSdlIn/9IuBaPNTe4oZA4H11h4ax0vS\n\trO1tdbiHIQzofUVuo90Zgm09zPSohzwQAq46GQiOUX8VBYyVTveZHEZF8x80lBmOa/7x\n\taj8qIVVtOB3/VotdqSHHz6rY/UQOsdhXL9sr8PdWzbrEmfTYhBSGW4xHV7MhEbLgs3dP\n\t57ag==","X-Gm-Message-State":"ANhLgQ3tyCQzGgmoMkn2oTu/y3PD45qVaRd0qMRxZOwEnoZqEEorgbJG\n\t0hVWtTKENAQagrpsFPyPy04=","X-Google-Smtp-Source":"ADFU+vtcCMpZcJVYpuMhqCf+R4hXJ2ymAqzvgr7Cov+h6vbWT36bfQb2os7fYfW+BbbaZOzvEhD46A==","X-Received":"by 2002:a2e:9395:: with SMTP id\n\tg21mr1722186ljh.167.1583223998999; \n\tTue, 03 Mar 2020 00:26:38 -0800 (PST)","Date":"Tue, 3 Mar 2020 10:26:27 +0200","From":"Pekka Paalanen <ppaalanen@gmail.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"dri-devel@lists.freedesktop.org, libcamera-devel@lists.libcamera.org,\n\tSakari Ailus <sakari.ailus@linux.intel.com>","Message-ID":"<20200303102627.2b726d83@eldfell.localdomain>","In-Reply-To":"<20200228163135.524882-1-niklas.soderlund@ragnatech.se>","References":"<20200228163135.524882-1-niklas.soderlund@ragnatech.se>","X-Mailer":"Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu)","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tboundary=\"Sig_/0mRuWl3=/EdOQuZs34/QxxB\";\n\tprotocol=\"application/pgp-signature\"","Subject":"Re: [libcamera-devel] [PATCH] drm/fourcc: Add bayer formats and\n\tmodifiers","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>","X-List-Received-Date":"Tue, 03 Mar 2020 08:26:40 -0000"}},{"id":3928,"web_url":"https://patchwork.libcamera.org/comment/3928/","msgid":"<20200303184218.GB13686@intel.com>","date":"2020-03-03T18:42:18","subject":"Re: [libcamera-devel] [PATCH] drm/fourcc: Add bayer formats and\n\tmodifiers","submitter":{"id":36,"url":"https://patchwork.libcamera.org/api/people/36/","name":"Ville Syrjälä","email":"ville.syrjala@linux.intel.com"},"content":"On Tue, Mar 03, 2020 at 10:26:27AM +0200, Pekka Paalanen wrote:\n> On Fri, 28 Feb 2020 17:31:35 +0100\n> Niklas Söderlund <niklas.soderlund@ragnatech.se> wrote:\n> \n> > Bayer formats are used with cameras and contain green, red and blue\n> > components, with alternating lines of red and green, and blue and green\n> > pixels in different orders. For each block of 2x2 pixels there is one\n> > pixel with a red filter, two with a green filter, and one with a blue\n> > filter. The filters can be arranged in different patterns.\n> > \n> > Add DRM fourcc formats to describe the most common Bayer formats. Also\n> > add a modifiers to describe the custom packing layouts used by the Intel\n> > IPU3 and in the MIPI (Mobile Industry Processor Interface) CSI-2\n> > specification.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  include/uapi/drm/drm_fourcc.h | 95 +++++++++++++++++++++++++++++++++++\n> >  1 file changed, 95 insertions(+)\n> \n> Hi,\n> \n> here are some by-stander comments.\n> \n> > \n> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h\n> > index 8bc0b31597d80737..561d5a08ffd16b69 100644\n> > --- a/include/uapi/drm/drm_fourcc.h\n> > +++ b/include/uapi/drm/drm_fourcc.h\n> > @@ -286,6 +286,62 @@ extern \"C\" {\n> >  #define DRM_FORMAT_YVU444\tfourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */\n> >  \n> >  \n> > +/*\n> > + * Bayer formats\n> > + *\n> > + * Bayer formats contain green, red and blue components, with alternating lines\n> > + * of red and green, and blue and green pixels in different orders. For each\n> > + * block of 2x2 pixels there is one pixel with a red filter, two with a green\n> > + * filter, and one with a blue filter. The filters can be arranged in different\n> > + * patterns.\n> > + *\n> > + * For example, RGGB:\n> > + *\trow0: RGRGRGRG...\n> > + *\trow1: GBGBGBGB...\n\nWhare is row2?\n\n> > + *\trow3: RGRGRGRG...\n> > + *\trow4: GBGBGBGB...\n> > + *\t...\n> > + *\n> > + * Vendors have different methods to pack the sampling formats to increase data\n> > + * density. For this reason the fourcc only describes pixel sample size and the\n> > + * filter pattern for each block of 2x2 pixels. A modifier is needed to\n> > + * describe the memory layout.\n> > + *\n> > + * In addition to vendor modifiers for memory layout DRM_FORMAT_MOD_LINEAR may\n> > + * be used to describe a layout where all samples are placed consecutively in\n> > + * memory. If the sample does not fit inside a single byte, the sample storage\n> > + * is extended to the minimum number of (little endian) bytes that can hold the\n> > + * sample and any unused most-significant bits are defined as padding.\n> \n> \"Minimum number of (little endian) bytes\" is probably not quite right,\n> because you could end up with a 3-byte word for e.g. 18-bit samples,\n> and for those I don't think endianess is even a defined concept.\n\nIn my book little-endian == \"little end comes first\". Nothing in that\ndefinition that says the number of bytes per unit has to a power of two.\n\nI guess maybe another way to put it would be to say \"each sample is\nstored as lsb aligned little endian value in minimum number of bytes\nrequired\". But some visual representation could help more I guess.\nWe try do that for the normal formats.\n\nThough I don't really know what \"samples are placed consecutively in\nmemory\" is trying to say here. Eg. for the row0-row4 example above\nwould we store this as RGRGRG...GBGBGB... or something more like\nRGGBRGGBRGGB..., or something else?\n\n\nSide note:\nSince people seem a bit confused by our use of \"little endian\" in\ngeneral I was thinking we should maybe update all the definitions to\nbe even more explicit. Something alogned the lines of:\n\"pixel [31:30 A][29:20 R][19:10 G][9:0   B]\n byte  [    3    ][    2    ][   1   ][ 0 ]\"\n\nThough the mismathed proportions make it rather ugly.\n\nMaybe we should even show the bit numbers for each component along\nwith everything else:\n\"component [1  A  0][9  R  4..3  R  0][9  G  6..5  G  0][9 B 8..7 B 0]\n pixel     [31 A 30][29 R 24..23 R 20][19 G 16..15 G 10][9 B 8..7 B 0]\n byte      [       3        ][       2        ][      1       ][  0  ]\"\n\nCould stretch the bytes to uniform size I guess. But still not entirely\nreadable :(\n\nAnyways, ideas for an actually good way to docuement formats welcome...","headers":{"Return-Path":"<ville.syrjala@linux.intel.com>","Received":["from mga11.intel.com (mga11.intel.com [192.55.52.93])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A80E627AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  3 Mar 2020 19:42:24 +0100 (CET)","from orsmga007.jf.intel.com ([10.7.209.58])\n\tby fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t03 Mar 2020 10:42:22 -0800","from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174])\n\tby orsmga007.jf.intel.com with SMTP; 03 Mar 2020 10:42:19 -0800","by stinkbox (sSMTP sendmail emulation);\n\tTue, 03 Mar 2020 20:42:18 +0200"],"X-Amp-Result":"UNKNOWN","X-Amp-Original-Verdict":"FILE UNKNOWN","X-Amp-File-Uploaded":"False","X-ExtLoop1":"1","X-IronPort-AV":"E=Sophos;i=\"5.70,511,1574150400\"; d=\"scan'208\";a=\"229027787\"","Date":"Tue, 3 Mar 2020 20:42:18 +0200","From":"Ville =?iso-8859-1?q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>","To":"Pekka Paalanen <ppaalanen@gmail.com>","Cc":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org, Sakari Ailus\n\t<sakari.ailus@linux.intel.com>,  dri-devel@lists.freedesktop.org","Message-ID":"<20200303184218.GB13686@intel.com>","References":"<20200228163135.524882-1-niklas.soderlund@ragnatech.se>\n\t<20200303102627.2b726d83@eldfell.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200303102627.2b726d83@eldfell.localdomain>","X-Patchwork-Hint":"comment","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] drm/fourcc: Add bayer formats and\n\tmodifiers","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>","X-List-Received-Date":"Tue, 03 Mar 2020 18:42:24 -0000"}},{"id":3929,"web_url":"https://patchwork.libcamera.org/comment/3929/","msgid":"<20200304173625.GM5379@paasikivi.fi.intel.com>","date":"2020-03-04T17:36:25","subject":"Re: [libcamera-devel] [PATCH] drm/fourcc: Add bayer formats and\n\tmodifiers","submitter":{"id":37,"url":"https://patchwork.libcamera.org/api/people/37/","name":"Sakari Ailus","email":"sakari.ailus@linux.intel.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, Feb 28, 2020 at 05:31:35PM +0100, Niklas Söderlund wrote:\n> Bayer formats are used with cameras and contain green, red and blue\n> components, with alternating lines of red and green, and blue and green\n> pixels in different orders. For each block of 2x2 pixels there is one\n> pixel with a red filter, two with a green filter, and one with a blue\n> filter. The filters can be arranged in different patterns.\n> \n> Add DRM fourcc formats to describe the most common Bayer formats. Also\n> add a modifiers to describe the custom packing layouts used by the Intel\n> IPU3 and in the MIPI (Mobile Industry Processor Interface) CSI-2\n> specification.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/uapi/drm/drm_fourcc.h | 95 +++++++++++++++++++++++++++++++++++\n>  1 file changed, 95 insertions(+)\n> \n> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h\n> index 8bc0b31597d80737..561d5a08ffd16b69 100644\n> --- a/include/uapi/drm/drm_fourcc.h\n> +++ b/include/uapi/drm/drm_fourcc.h\n> @@ -286,6 +286,62 @@ extern \"C\" {\n>  #define DRM_FORMAT_YVU444\tfourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */\n>  \n>  \n> +/*\n> + * Bayer formats\n> + *\n> + * Bayer formats contain green, red and blue components, with alternating lines\n> + * of red and green, and blue and green pixels in different orders. For each\n> + * block of 2x2 pixels there is one pixel with a red filter, two with a green\n> + * filter, and one with a blue filter. The filters can be arranged in different\n> + * patterns.\n> + *\n> + * For example, RGGB:\n> + *\trow0: RGRGRGRG...\n> + *\trow1: GBGBGBGB...\n> + *\trow3: RGRGRGRG...\n> + *\trow4: GBGBGBGB...\n> + *\t...\n> + *\n> + * Vendors have different methods to pack the sampling formats to increase data\n> + * density. For this reason the fourcc only describes pixel sample size and the\n\nThis could be for other reasons than data density, such as for less\ncumbersome memory access. I'd leave out \"to increase data density\".\n\n> + * filter pattern for each block of 2x2 pixels. A modifier is needed to\n> + * describe the memory layout.\n> + *\n> + * In addition to vendor modifiers for memory layout DRM_FORMAT_MOD_LINEAR may\n> + * be used to describe a layout where all samples are placed consecutively in\n> + * memory. If the sample does not fit inside a single byte, the sample storage\n> + * is extended to the minimum number of (little endian) bytes that can hold the\n> + * sample and any unused most-significant bits are defined as padding.\n\nShould it be added here that padding bits are all zero?\n\n> + *\n> + * For example, SRGGB10:\n> + * Each 10-bit sample is contained in 2 consecutive little endian bytes, where\n> + * the 6 most-significant bits are unused.\n> + */\n> +\n> +/* 8-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB8\tfourcc_code('R', 'G', 'G', 'B')\n> +#define DRM_FORMAT_SGRBG8\tfourcc_code('G', 'R', 'B', 'G')\n> +#define DRM_FORMAT_SGBRG8\tfourcc_code('G', 'B', 'R', 'G')\n> +#define DRM_FORMAT_SBGGR8\tfourcc_code('B', 'A', '8', '1')\n> +\n> +/* 10-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB10\tfourcc_code('R', 'G', '1', '0')\n> +#define DRM_FORMAT_SGRBG10\tfourcc_code('B', 'A', '1', '0')\n> +#define DRM_FORMAT_SGBRG10\tfourcc_code('G', 'B', '1', '0')\n> +#define DRM_FORMAT_SBGGR10\tfourcc_code('B', 'G', '1', '0')\n> +\n> +/* 12-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB12\tfourcc_code('R', 'G', '1', '2')\n> +#define DRM_FORMAT_SGRBG12\tfourcc_code('B', 'A', '1', '2')\n> +#define DRM_FORMAT_SGBRG12\tfourcc_code('G', 'B', '1', '2')\n> +#define DRM_FORMAT_SBGGR12\tfourcc_code('B', 'G', '1', '2')\n> +\n> +/* 14-bit Bayer formats */\n> +#define DRM_FORMAT_SRGGB14\tfourcc_code('R', 'G', '1', '4')\n> +#define DRM_FORMAT_SGRBG14\tfourcc_code('B', 'A', '1', '4')\n> +#define DRM_FORMAT_SGBRG14\tfourcc_code('G', 'B', '1', '4')\n> +#define DRM_FORMAT_SBGGR14\tfourcc_code('B', 'G', '1', '4')\n\nThe 4cc codes seemingly appear to those used in V4L2. Is that intentional?\nNothing wrong in that though, but is this a rule that's universally\nfollowed?\n\n> +\n>  /*\n>   * Format Modifiers:\n>   *\n> @@ -309,6 +365,7 @@ extern \"C\" {\n>  #define DRM_FORMAT_MOD_VENDOR_BROADCOM 0x07\n>  #define DRM_FORMAT_MOD_VENDOR_ARM     0x08\n>  #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> +#define DRM_FORMAT_MOD_VENDOR_MIPI 0x0a\n>  \n>  /* add more to the end as needed */\n>  \n> @@ -434,6 +491,17 @@ extern \"C\" {\n>   */\n>  #define I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS fourcc_mod_code(INTEL, 7)\n>  \n> +\n> +/*\n> + * IPU3 Bayer packing layout\n> + *\n> + * The IPU3 raw Bayer formats use a custom packing layout where there are no\n> + * gaps between each 10-bit sample. It packs 25 pixels into 32 bytes leaving\n> + * the 6 most significant bits in the last byte unused. The format is little\n> + * endian.\n\nWhere is endianness really specified? DRM_FORMAT_BIG_ENDIAN would seem\nindependent of that.\n\nI might omit endianness definition here.\n\n> + */\n> +#define IPU3_FORMAT_MOD_PACKED fourcc_mod_code(INTEL, 8)\n> +\n>  /*\n>   * Tiled, NV12MT, grouped in 64 (pixels) x 32 (lines) -sized macroblocks\n>   *\n> @@ -804,6 +872,33 @@ extern \"C\" {\n>   */\n>  #define DRM_FORMAT_MOD_ALLWINNER_TILED fourcc_mod_code(ALLWINNER, 1)\n>  \n> +/* Mobile Industry Processor Interface (MIPI) modifiers */\n> +\n> +/*\n> + * MIPI CSI-2 packing layout\n> + *\n> + * The CSI-2 RAW formats (for example Bayer) use a different packing layout\n> + * depenindg on the sample size.\n\n\"depending\"\n\n> + *\n> + * - 10-bits per sample\n> + *   Every four consecutive samples are packed into 5 bytes. Each of the first 4\n> + *   bytes contain the 8 high order bits of the pixels, and the 5th byte\n> + *   contains the 2 least-significant bits of each pixel, in the same order.\n> + *\n> + * - 12-bits per sample\n> + *   Every two consecutive samples are packed into three bytes. Each of the\n> + *   first two bytes contain the 8 high order bits of the pixels, and the third\n> + *   byte contains the four least-significant bits of each pixel, in the same\n> + *   order.\n> + *\n> + * - 14-bits per sample\n> + *   Every four consecutive samples are packed into seven bytes. Each of the\n> + *   first four bytes contain the eight high order bits of the pixels, and the\n> + *   three following bytes contains the six least-significant bits of each\n> + *   pixel, in the same order.\n> + */\n> +#define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n> +\n>  #if defined(__cplusplus)\n>  }\n>  #endif","headers":{"Return-Path":"<sakari.ailus@linux.intel.com>","Received":["from mga06.intel.com (mga06.intel.com [134.134.136.31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BD3EB60426\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Mar 2020 18:36:36 +0100 (CET)","from orsmga008.jf.intel.com ([10.7.209.65])\n\tby orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t04 Mar 2020 09:36:29 -0800","from paasikivi.fi.intel.com ([10.237.72.42])\n\tby orsmga008-auth.jf.intel.com with\n\tESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Mar 2020 09:36:28 -0800","by paasikivi.fi.intel.com (Postfix, from userid 1000)\n\tid CE78C209DF; Wed,  4 Mar 2020 19:36:25 +0200 (EET)"],"X-Amp-Result":"UNKNOWN","X-Amp-Original-Verdict":"FILE UNKNOWN","X-Amp-File-Uploaded":"False","X-IronPort-AV":"E=Sophos;i=\"5.70,514,1574150400\"; d=\"scan'208\";a=\"234109202\"","Date":"Wed, 4 Mar 2020 19:36:25 +0200","From":"Sakari Ailus <sakari.ailus@linux.intel.com>","To":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"dri-devel@lists.freedesktop.org, libcamera-devel@lists.libcamera.org","Message-ID":"<20200304173625.GM5379@paasikivi.fi.intel.com>","References":"<20200228163135.524882-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200228163135.524882-1-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] drm/fourcc: Add bayer formats and\n\tmodifiers","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>","X-List-Received-Date":"Wed, 04 Mar 2020 17:36:37 -0000"}}]