[PATCH 0/8] kmsxx: Various fixes and improvements

[PATCH 0/8] kmsxx: Various fixes and improvements

[Laurent Pinchart]

Hi Tomi,

This patch series fixes a few issues in kmsxx, mainly related to support
for additional pixel formats.

Patch 1/8 is a small API extension proposal to expose the card device
minor number, and patch 2/8 is a typo fix in a method name.

The remaining patches add support for additional RGB and YUV formats,
fixing a few issues in the dumb frame buffer class for fully planar
buffers (patches 3/8 and 4/8), and adding support for the new formats in
patches 5/8 to 8/8.

All the new formats have been tested on a Renesas R-Car Gen3 device,
which supports

- PixelFormat.RGB332
- PixelFormat.ARGB4444
- PixelFormat.XRGB4444
- PixelFormat.ARGB1555
- PixelFormat.XRGB1555
- PixelFormat.RGB565
- PixelFormat.BGR888
- PixelFormat.RGB888
- PixelFormat.BGRA8888
- PixelFormat.BGRX8888
- PixelFormat.ARGB8888
- PixelFormat.XRGB8888
- PixelFormat.UYVY
- PixelFormat.YUYV
- PixelFormat.YVYU
- PixelFormat.NV12
- PixelFormat.NV21
- PixelFormat.NV16
- PixelFormat.NV61
- PixelFormat.YUV420
- PixelFormat.YVU420
- PixelFormat.YUV422
- PixelFormat.YVU422
- PixelFormat.YUV444
- PixelFormat.YVU444

Laurent Pinchart (8):
  card: Add a method to retrieve the device minor
  card: Rename has_has_universal_planes to has_universal_planes
  dumbfb: Add support tri- or quadri-planar buffers
  dumbfb: Fix pitch for tri-planar formats
  pykmsbase: Add missing pixel formats
  kms++: Add support for semiplanar YUV422 formats (NV16 and NV61)
  kms++: Add support for the planar YUV formats
  kms++: Add support for missing 8 -and 16-bit RGB formats

 kms++/inc/kms++/card.h              |   5 +-
 kms++/inc/kms++/pixelformats.h      |  14 ++
 kms++/src/card.cpp                  |  11 ++
 kms++/src/dumbframebuffer.cpp       |  23 ++-
 kms++/src/pixelformats.cpp          |  18 +-
 kms++/src/plane.cpp                 |   2 +-
 kms++util/inc/kms++util/color.h     |   1 +
 kms++util/inc/kms++util/kms++util.h |   1 +
 kms++util/src/color.cpp             |   5 +
 kms++util/src/drawing.cpp           | 279 +++++++++++++++++++++++++---
 kms++util/src/testpat.cpp           |  16 +-
 py/pykms/pykmsbase.cpp              |  33 ++++
 utils/kmsprint.cpp                  |   4 +-
 13 files changed, 370 insertions(+), 42 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/8] card: Add a method to retrieve the device minor

[Laurent Pinchart]

The device minor number is needed to access the debugfs directory
corresponding to the device. Make it available to users through a
get_minor() method on the Card object.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/inc/kms++/card.h |  3 +++
 kms++/src/card.cpp     | 11 +++++++++++
 py/pykms/pykmsbase.cpp |  1 +
 3 files changed, 15 insertions(+)

diff --git a/kms++/inc/kms++/card.h b/kms++/inc/kms++/card.h
index 5c1cf7cfcedc..0a11747f7985 100644
--- a/kms++/inc/kms++/card.h
+++ b/kms++/inc/kms++/card.h
@@ -35,6 +35,7 @@ public:
 	Card& operator=(const Card& other) = delete;
 
 	int fd() const { return m_fd; }
+	unsigned int dev_minor() const { return m_minor; }
 
 	void drop_master();
 
@@ -84,7 +85,9 @@ private:
 	std::vector<Framebuffer*> m_framebuffers;
 
 	int m_fd;
+	unsigned int m_minor;
 	bool m_is_master;
+	std::string m_device;
 
 	bool m_has_atomic;
 	bool m_has_universal_planes;
diff --git a/kms++/src/card.cpp b/kms++/src/card.cpp
index 527aca6cd127..3a7ab700ed49 100644
--- a/kms++/src/card.cpp
+++ b/kms++/src/card.cpp
@@ -9,6 +9,10 @@
 #include <algorithm>
 #include <glob.h>
 
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#include <sys/types.h>
+
 #include <xf86drm.h>
 #include <xf86drmMode.h>
 
@@ -183,8 +187,15 @@ void Card::setup()
 	m_version.desc = string(ver->desc, ver->desc_len);
 	drmFreeVersion(ver);
 
+	struct stat stats;
 	int r;
 
+	r = fstat(m_fd, &stats);
+	if (r < 0)
+		throw invalid_argument("Can't stat device (" + string(strerror(errno)) + ")");
+
+	m_minor = minor(stats.st_dev);
+
 	r = drmSetMaster(m_fd);
 	m_is_master = r == 0;
 
diff --git a/py/pykms/pykmsbase.cpp b/py/pykms/pykmsbase.cpp
index c039833a41fb..fc72d056627f 100644
--- a/py/pykms/pykmsbase.cpp
+++ b/py/pykms/pykmsbase.cpp
@@ -24,6 +24,7 @@ void init_pykmsbase(py::module &m)
 			.def(py::init<const string&>())
 			.def(py::init<const string&, uint32_t>())
 			.def_property_readonly("fd", &Card::fd)
+			.def_property_readonly("minor", &Card::dev_minor)
 			.def_property_readonly("get_first_connected_connector", &Card::get_first_connected_connector)
 
 			// XXX pybind11 can't handle vector<T*> where T is non-copyable, and complains:
-- 
Regards,

Laurent Pinchart

[PATCH 2/8] card: Rename has_has_universal_planes to has_universal_planes

[Laurent Pinchart]

The has_has_universal_planes() method name includes a typo, fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/inc/kms++/card.h | 2 +-
 kms++/src/plane.cpp    | 2 +-
 utils/kmsprint.cpp     | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kms++/inc/kms++/card.h b/kms++/inc/kms++/card.h
index 0a11747f7985..e9f90e3a1f96 100644
--- a/kms++/inc/kms++/card.h
+++ b/kms++/inc/kms++/card.h
@@ -50,7 +50,7 @@ public:
 
 	bool is_master() const { return m_is_master; }
 	bool has_atomic() const { return m_has_atomic; }
-	bool has_has_universal_planes() const { return m_has_universal_planes; }
+	bool has_universal_planes() const { return m_has_universal_planes; }
 	bool has_dumb_buffers() const { return m_has_dumb; }
 	bool has_kms() const;
 
diff --git a/kms++/src/plane.cpp b/kms++/src/plane.cpp
index b04088492c92..2df51c1ccdf8 100644
--- a/kms++/src/plane.cpp
+++ b/kms++/src/plane.cpp
@@ -51,7 +51,7 @@ bool Plane::supports_format(PixelFormat fmt) const
 
 PlaneType Plane::plane_type() const
 {
-	if (card().has_has_universal_planes()) {
+	if (card().has_universal_planes()) {
 		switch (get_prop_value("type")) {
 		case DRM_PLANE_TYPE_OVERLAY:
 			return PlaneType::Overlay;
diff --git a/utils/kmsprint.cpp b/utils/kmsprint.cpp
index 807aa62825ac..9d3d7ccfd9d4 100644
--- a/utils/kmsprint.cpp
+++ b/utils/kmsprint.cpp
@@ -372,7 +372,7 @@ static void print_as_list(Card& card)
 
 	for (Crtc* crtc : card.get_crtcs()) {
 		obs.push_back(crtc);
-		if (crtc->buffer_id() && !card.has_has_universal_planes()) {
+		if (crtc->buffer_id() && !card.has_universal_planes()) {
 			Framebuffer* fb = new Framebuffer(card, crtc->buffer_id());
 			fbs.push_back(fb);
 		}
@@ -423,7 +423,7 @@ static void print_as_tree(Card& card)
 				if (s_opts.print_props)
 					e3.lines = format_props(crtc);
 
-				if (crtc->buffer_id() && !card.has_has_universal_planes()) {
+				if (crtc->buffer_id() && !card.has_universal_planes()) {
 					Framebuffer fb(card, crtc->buffer_id());
 					Entry& e5 = add_entry(e3.children);
 
-- 
Regards,

Laurent Pinchart

[PATCH 3/8] dumbfb: Add support tri- or quadri-planar buffers

[Laurent Pinchart]

The DumbFrameBuffer class supports up to 4 planes, as required by the
DRM/KMS API, but only considers planes 0 and 1 when constructing the
buffer. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/src/dumbframebuffer.cpp | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/kms++/src/dumbframebuffer.cpp b/kms++/src/dumbframebuffer.cpp
index 3448fb149df2..18f3f152943d 100644
--- a/kms++/src/dumbframebuffer.cpp
+++ b/kms++/src/dumbframebuffer.cpp
@@ -56,9 +56,18 @@ DumbFramebuffer::DumbFramebuffer(Card& card, uint32_t width, uint32_t height, Pi
 	}
 
 	/* create framebuffer object for the dumb-buffer */
-	uint32_t bo_handles[4] = { m_planes[0].handle, m_planes[1].handle };
-	uint32_t pitches[4] = { m_planes[0].stride, m_planes[1].stride };
-	uint32_t offsets[4] = {  m_planes[0].offset, m_planes[1].offset };
+	uint32_t bo_handles[4] = {
+		m_planes[0].handle, m_planes[1].handle,
+		m_planes[2].handle, m_planes[3].handle,
+	};
+	uint32_t pitches[4] = {
+		m_planes[0].stride, m_planes[1].stride,
+		m_planes[2].stride, m_planes[3].stride,
+	};
+	uint32_t offsets[4] = {
+		m_planes[0].offset, m_planes[1].offset,
+		m_planes[2].offset, m_planes[3].offset,
+	};
 	uint32_t id;
 	r = drmModeAddFB2(card.fd(), width, height, (uint32_t)format,
 			  bo_handles, pitches, offsets, &id, 0);
-- 
Regards,

Laurent Pinchart

[PATCH 4/8] dumbfb: Fix pitch for tri-planar formats

[Laurent Pinchart]

The BO pitches are unconditionally set to the frame buffer pitch, for
all planes. This is correct for semiplanar YUV formats, as they
subsample chroma horizontally by two but combined U and V in a single
plane, cancelling each other. For fully planar YUV formats, however, the
horizontal subsampling need to be taken into account to compute the
pitch. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/src/dumbframebuffer.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kms++/src/dumbframebuffer.cpp b/kms++/src/dumbframebuffer.cpp
index 18f3f152943d..4c3c03164a90 100644
--- a/kms++/src/dumbframebuffer.cpp
+++ b/kms++/src/dumbframebuffer.cpp
@@ -42,6 +42,14 @@ DumbFramebuffer::DumbFramebuffer(Card& card, uint32_t width, uint32_t height, Pi
 		struct drm_mode_create_dumb creq = drm_mode_create_dumb();
 		creq.width = width;
 		creq.height = height / pi.ysub;
+		/*
+		 * For fully planar YUV buffers, the chroma planes don't combine
+		 * U and V components, their width must thus be divided by the
+		 * horizontal subsampling factor.
+		 */
+		if (format_info.type == PixelColorType::YUV &&
+		    format_info.num_planes == 3)
+			creq.width /= pi.xsub;
 		creq.bpp = pi.bitspp;
 		r = drmIoctl(card.fd(), DRM_IOCTL_MODE_CREATE_DUMB, &creq);
 		if (r)
-- 
Regards,

Laurent Pinchart

[PATCH 5/8] pykmsbase: Add missing pixel formats

[Laurent Pinchart]

Several pixel formats defined in the C++ PixelFormat class are missing
from the Python API. Add them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 py/pykms/pykmsbase.cpp | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/py/pykms/pykmsbase.cpp b/py/pykms/pykmsbase.cpp
index fc72d056627f..407b9485f556 100644
--- a/py/pykms/pykmsbase.cpp
+++ b/py/pykms/pykmsbase.cpp
@@ -186,14 +186,32 @@ void init_pykmsbase(py::module &m)
 
 			.value("XRGB8888", PixelFormat::XRGB8888)
 			.value("XBGR8888", PixelFormat::XBGR8888)
+			.value("RGBX8888", PixelFormat::RGBX8888)
+			.value("BGRX8888", PixelFormat::BGRX8888)
+
 			.value("ARGB8888", PixelFormat::ARGB8888)
 			.value("ABGR8888", PixelFormat::ABGR8888)
+			.value("RGBA8888", PixelFormat::RGBA8888)
+			.value("BGRA8888", PixelFormat::BGRA8888)
 
 			.value("RGB888", PixelFormat::RGB888)
 			.value("BGR888", PixelFormat::BGR888)
 
 			.value("RGB565", PixelFormat::RGB565)
 			.value("BGR565", PixelFormat::BGR565)
+
+			.value("ARGB4444", PixelFormat::ARGB4444)
+			.value("ARGB1555", PixelFormat::ARGB1555)
+
+			.value("XRGB2101010", PixelFormat::XRGB2101010)
+			.value("XBGR2101010", PixelFormat::XBGR2101010)
+			.value("RGBX1010102", PixelFormat::RGBX1010102)
+			.value("BGRX1010102", PixelFormat::BGRX1010102)
+
+			.value("ARGB2101010", PixelFormat::ARGB2101010)
+			.value("ABGR2101010", PixelFormat::ABGR2101010)
+			.value("RGBA1010102", PixelFormat::RGBA1010102)
+			.value("BGRA1010102", PixelFormat::BGRA1010102)
 			;
 
 	py::enum_<SyncPolarity>(m, "SyncPolarity")
-- 
Regards,

Laurent Pinchart

[PATCH 6/8] kms++: Add support for semiplanar YUV422 formats (NV16 and NV61)

[Laurent Pinchart]

Add support for the NV16 and NV61 pixel formats to the PixelFormat
class, the Python API, and the drawing utilities.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/inc/kms++/pixelformats.h |  2 +
 kms++/src/pixelformats.cpp     |  2 +
 kms++util/src/drawing.cpp      | 68 +++++++++++++++++++++++++++++++---
 kms++util/src/testpat.cpp      |  7 ++--
 py/pykms/pykmsbase.cpp         |  2 +
 5 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/kms++/inc/kms++/pixelformats.h b/kms++/inc/kms++/pixelformats.h
index 4e73d5dddb18..f550f2fff4e0 100644
--- a/kms++/inc/kms++/pixelformats.h
+++ b/kms++/inc/kms++/pixelformats.h
@@ -17,6 +17,8 @@ enum class PixelFormat : uint32_t
 
 	NV12 = MakeFourCC("NV12"),
 	NV21 = MakeFourCC("NV21"),
+	NV16 = MakeFourCC("NV16"),
+	NV61 = MakeFourCC("NV61"),
 
 	UYVY = MakeFourCC("UYVY"),
 	YUYV = MakeFourCC("YUYV"),
diff --git a/kms++/src/pixelformats.cpp b/kms++/src/pixelformats.cpp
index ecca41d7fa00..d34df6435436 100644
--- a/kms++/src/pixelformats.cpp
+++ b/kms++/src/pixelformats.cpp
@@ -15,6 +15,8 @@ static const map<PixelFormat, PixelFormatInfo> format_info_array = {
 	/* YUV semi-planar */
 	{ PixelFormat::NV12, { PixelColorType::YUV, 2, { { 8, 1, 1, }, { 8, 2, 2 } }, } },
 	{ PixelFormat::NV21, { PixelColorType::YUV, 2, { { 8, 1, 1, }, { 8, 2, 2 } }, } },
+	{ PixelFormat::NV16, { PixelColorType::YUV, 2, { { 8, 1, 1, }, { 8, 2, 1 } }, } },
+	{ PixelFormat::NV61, { PixelColorType::YUV, 2, { { 8, 1, 1, }, { 8, 2, 1 } }, } },
 	/* RGB16 */
 	{ PixelFormat::RGB565, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
 	{ PixelFormat::BGR565, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
diff --git a/kms++util/src/drawing.cpp b/kms++util/src/drawing.cpp
index 194daf876586..a329b8362072 100644
--- a/kms++util/src/drawing.cpp
+++ b/kms++util/src/drawing.cpp
@@ -115,13 +115,9 @@ void draw_rgb_pixel(IFramebuffer& buf, unsigned x, unsigned y, RGB color)
 	}
 }
 
-void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1, YUV yuv2)
+static void draw_yuv422_packed_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
+					  YUV yuv1, YUV yuv2)
 {
-	if ((x + 1) >= buf.width() || y >= buf.height())
-		throw runtime_error("attempt to draw outside the buffer");
-
-	ASSERT((x & 1) == 0);
-
 	uint8_t *p = (uint8_t*)(buf.map(0) + buf.stride(0) * y + x * 2);
 
 	uint8_t y0 = yuv1.y;
@@ -163,6 +159,62 @@ void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1,
 	}
 }
 
+static void draw_yuv422_semiplanar_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
+					      YUV yuv1, YUV yuv2)
+{
+	uint8_t *py = (uint8_t*)(buf.map(0) + buf.stride(0) * y + x);
+	uint8_t *puv = (uint8_t*)(buf.map(1) + buf.stride(1) * y + x);
+
+	uint8_t y0 = yuv1.y;
+	uint8_t y1 = yuv2.y;
+	uint8_t u = (yuv1.u + yuv2.u) / 2;
+	uint8_t v = (yuv1.v + yuv2.v) / 2;
+
+	switch (buf.format()) {
+	case PixelFormat::NV16:
+		py[0] = y0;
+		py[1] = y1;
+		puv[0] = u;
+		puv[1] = v;
+		break;
+
+	case PixelFormat::NV61:
+		py[0] = y0;
+		py[1] = y1;
+		puv[0] = v;
+		puv[1] = u;
+		break;
+
+	default:
+		throw std::invalid_argument("invalid pixelformat");
+	}
+}
+
+void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1, YUV yuv2)
+{
+	if ((x + 1) >= buf.width() || y >= buf.height())
+		throw runtime_error("attempt to draw outside the buffer");
+
+	ASSERT((x & 1) == 0);
+
+	switch (buf.format()) {
+	case PixelFormat::UYVY:
+	case PixelFormat::YUYV:
+	case PixelFormat::YVYU:
+	case PixelFormat::VYUY:
+		draw_yuv422_packed_macropixel(buf, x, y, yuv1, yuv2);
+		break;
+
+	case PixelFormat::NV16:
+	case PixelFormat::NV61:
+		draw_yuv422_semiplanar_macropixel(buf, x, y, yuv1, yuv2);
+		break;
+
+	default:
+		throw std::invalid_argument("invalid pixelformat");
+	}
+}
+
 void draw_yuv420_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
 			    YUV yuv1, YUV yuv2, YUV yuv3, YUV yuv4)
 {
@@ -235,6 +287,8 @@ void draw_rect(IFramebuffer &fb, uint32_t x, uint32_t y, uint32_t w, uint32_t h,
 	case PixelFormat::YUYV:
 	case PixelFormat::YVYU:
 	case PixelFormat::VYUY:
+	case PixelFormat::NV16:
+	case PixelFormat::NV61:
 		for (j = 0; j < h; j++) {
 			for (i = 0; i < w; i += 2) {
 				draw_yuv422_macropixel(fb, x + i, y + j, yuvcolor, yuvcolor);
@@ -311,6 +365,8 @@ static void draw_char(IFramebuffer& buf, uint32_t xpos, uint32_t ypos, char c, R
 	case PixelFormat::YUYV:
 	case PixelFormat::YVYU:
 	case PixelFormat::VYUY:
+	case PixelFormat::NV16:
+	case PixelFormat::NV61:
 		for (y = 0; y < 8; y++) {
 			for (x = 0; x < 8; x += 2) {
 				bool b0 = get_char_pixel(c, x, y);
diff --git a/kms++util/src/testpat.cpp b/kms++util/src/testpat.cpp
index f9a3c8a025cd..c120de34a101 100644
--- a/kms++util/src/testpat.cpp
+++ b/kms++util/src/testpat.cpp
@@ -106,6 +106,7 @@ static void draw_test_pattern_part(IFramebuffer& fb, unsigned start_y, unsigned
 	unsigned w = fb.width();
 
 	const PixelFormatInfo& format_info = get_pixel_format_info(fb.format());
+	const PixelFormatPlaneInfo& plane_info = format_info.planes[format_info.num_planes - 1];
 
 	switch (format_info.type) {
 	case PixelColorType::RGB:
@@ -118,8 +119,8 @@ static void draw_test_pattern_part(IFramebuffer& fb, unsigned start_y, unsigned
 		break;
 
 	case PixelColorType::YUV:
-		switch (format_info.num_planes) {
-		case 1:
+		switch (plane_info.xsub + plane_info.ysub) {
+		case 3:
 			for (y = start_y; y < end_y; y++) {
 				for (x = 0; x < w; x += 2) {
 					RGB pixel1 = get_test_pattern_pixel(fb, x, y);
@@ -129,7 +130,7 @@ static void draw_test_pattern_part(IFramebuffer& fb, unsigned start_y, unsigned
 			}
 			break;
 
-		case 2:
+		case 4:
 			for (y = start_y; y < end_y; y += 2) {
 				for (x = 0; x < w; x += 2) {
 					RGB pixel00 = get_test_pattern_pixel(fb, x, y);
diff --git a/py/pykms/pykmsbase.cpp b/py/pykms/pykmsbase.cpp
index 407b9485f556..003ad3c1a0b2 100644
--- a/py/pykms/pykmsbase.cpp
+++ b/py/pykms/pykmsbase.cpp
@@ -178,6 +178,8 @@ void init_pykmsbase(py::module &m)
 
 			.value("NV12", PixelFormat::NV12)
 			.value("NV21", PixelFormat::NV21)
+			.value("NV16", PixelFormat::NV16)
+			.value("NV61", PixelFormat::NV61)
 
 			.value("UYVY", PixelFormat::UYVY)
 			.value("YUYV", PixelFormat::YUYV)
-- 
Regards,

Laurent Pinchart

[PATCH 7/8] kms++: Add support for the planar YUV formats

[Laurent Pinchart]

Add support for the 6 planar YUV formats (YUV and YVU, combined with
420, 422 or 444 subsampling) to the PixelFormat class, the Python API,
and the drawing utilities.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/inc/kms++/pixelformats.h      |   7 +
 kms++/src/pixelformats.cpp          |   7 +
 kms++util/inc/kms++util/kms++util.h |   1 +
 kms++util/src/drawing.cpp           | 203 ++++++++++++++++++++++++----
 kms++util/src/testpat.cpp           |   9 ++
 py/pykms/pykmsbase.cpp              |   7 +
 6 files changed, 208 insertions(+), 26 deletions(-)

diff --git a/kms++/inc/kms++/pixelformats.h b/kms++/inc/kms++/pixelformats.h
index f550f2fff4e0..746f9e2b8c5a 100644
--- a/kms++/inc/kms++/pixelformats.h
+++ b/kms++/inc/kms++/pixelformats.h
@@ -20,6 +20,13 @@ enum class PixelFormat : uint32_t
 	NV16 = MakeFourCC("NV16"),
 	NV61 = MakeFourCC("NV61"),
 
+	YUV420 = MakeFourCC("YU12"),
+	YVU420 = MakeFourCC("YV12"),
+	YUV422 = MakeFourCC("YU16"),
+	YVU422 = MakeFourCC("YV16"),
+	YUV444 = MakeFourCC("YU24"),
+	YVU444 = MakeFourCC("YV24"),
+
 	UYVY = MakeFourCC("UYVY"),
 	YUYV = MakeFourCC("YUYV"),
 	YVYU = MakeFourCC("YVYU"),
diff --git a/kms++/src/pixelformats.cpp b/kms++/src/pixelformats.cpp
index d34df6435436..fe86fca04cd8 100644
--- a/kms++/src/pixelformats.cpp
+++ b/kms++/src/pixelformats.cpp
@@ -17,6 +17,13 @@ static const map<PixelFormat, PixelFormatInfo> format_info_array = {
 	{ PixelFormat::NV21, { PixelColorType::YUV, 2, { { 8, 1, 1, }, { 8, 2, 2 } }, } },
 	{ PixelFormat::NV16, { PixelColorType::YUV, 2, { { 8, 1, 1, }, { 8, 2, 1 } }, } },
 	{ PixelFormat::NV61, { PixelColorType::YUV, 2, { { 8, 1, 1, }, { 8, 2, 1 } }, } },
+	/* YUV planar */
+	{ PixelFormat::YUV420, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 2, 2 }, { 8, 2, 2 } }, } },
+	{ PixelFormat::YVU420, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 2, 2 }, { 8, 2, 2 } }, } },
+	{ PixelFormat::YUV422, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 2, 1 }, { 8, 2, 1 } }, } },
+	{ PixelFormat::YVU422, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 2, 1 }, { 8, 2, 1 } }, } },
+	{ PixelFormat::YUV444, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 1, 1 }, { 8, 1, 1 } }, } },
+	{ PixelFormat::YVU444, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 1, 1 }, { 8, 1, 1 } }, } },
 	/* RGB16 */
 	{ PixelFormat::RGB565, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
 	{ PixelFormat::BGR565, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
diff --git a/kms++util/inc/kms++util/kms++util.h b/kms++util/inc/kms++util/kms++util.h
index 62ec663725eb..8fc6c8b81e48 100644
--- a/kms++util/inc/kms++util/kms++util.h
+++ b/kms++util/inc/kms++util/kms++util.h
@@ -18,6 +18,7 @@ namespace kms
 class IFramebuffer;
 
 void draw_rgb_pixel(IFramebuffer& buf, unsigned x, unsigned y, RGB color);
+void draw_yuv444_pixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv);
 void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1, YUV yuv2);
 void draw_yuv420_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
 				   YUV yuv1, YUV yuv2, YUV yuv3, YUV yuv4);
diff --git a/kms++util/src/drawing.cpp b/kms++util/src/drawing.cpp
index a329b8362072..c1c639a8b3a5 100644
--- a/kms++util/src/drawing.cpp
+++ b/kms++util/src/drawing.cpp
@@ -115,6 +115,33 @@ void draw_rgb_pixel(IFramebuffer& buf, unsigned x, unsigned y, RGB color)
 	}
 }
 
+void draw_yuv444_pixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv)
+{
+	if (x >= buf.width() || y >= buf.height())
+		throw runtime_error("attempt to draw outside the buffer");
+
+	uint8_t *py = (uint8_t*)(buf.map(0) + buf.stride(0) * y + x);
+	uint8_t *pu = (uint8_t*)(buf.map(1) + buf.stride(1) * y + x);
+	uint8_t *pv = (uint8_t*)(buf.map(2) + buf.stride(2) * y + x);
+
+	switch (buf.format()) {
+	case PixelFormat::YUV444:
+		py[0] = yuv.y;
+		pu[0] = yuv.u;
+		pv[0] = yuv.v;
+		break;
+
+	case PixelFormat::YVU444:
+		py[0] = yuv.y;
+		pu[0] = yuv.v;
+		pv[0] = yuv.u;
+		break;
+
+	default:
+		throw std::invalid_argument("invalid pixelformat");
+	}
+}
+
 static void draw_yuv422_packed_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
 					  YUV yuv1, YUV yuv2)
 {
@@ -190,6 +217,38 @@ static void draw_yuv422_semiplanar_macropixel(IFramebuffer& buf, unsigned x, uns
 	}
 }
 
+static void draw_yuv422_planar_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
+					  YUV yuv1, YUV yuv2)
+{
+	uint8_t *py = (uint8_t*)(buf.map(0) + buf.stride(0) * y + x);
+	uint8_t *pu = (uint8_t*)(buf.map(1) + buf.stride(1) * y + x / 2);
+	uint8_t *pv = (uint8_t*)(buf.map(2) + buf.stride(2) * y + x / 2);
+
+	uint8_t y0 = yuv1.y;
+	uint8_t y1 = yuv2.y;
+	uint8_t u = (yuv1.u + yuv2.u) / 2;
+	uint8_t v = (yuv1.v + yuv2.v) / 2;
+
+	switch (buf.format()) {
+	case PixelFormat::YUV422:
+		py[0] = y0;
+		py[1] = y1;
+		pu[0] = u;
+		pv[0] = v;
+		break;
+
+	case PixelFormat::YVU422:
+		py[0] = y0;
+		py[1] = y1;
+		pu[0] = v;
+		pv[0] = u;
+		break;
+
+	default:
+		throw std::invalid_argument("invalid pixelformat");
+	}
+}
+
 void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1, YUV yuv2)
 {
 	if ((x + 1) >= buf.width() || y >= buf.height())
@@ -210,6 +269,90 @@ void draw_yuv422_macropixel(IFramebuffer& buf, unsigned x, unsigned y, YUV yuv1,
 		draw_yuv422_semiplanar_macropixel(buf, x, y, yuv1, yuv2);
 		break;
 
+	case PixelFormat::YUV422:
+	case PixelFormat::YVU422:
+		draw_yuv422_planar_macropixel(buf, x, y, yuv1, yuv2);
+		break;
+
+	default:
+		throw std::invalid_argument("invalid pixelformat");
+	}
+}
+
+static void draw_yuv420_semiplanar_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
+					      YUV yuv1, YUV yuv2, YUV yuv3, YUV yuv4)
+{
+	uint8_t *py1 = (uint8_t*)(buf.map(0) + buf.stride(0) * (y + 0) + x);
+	uint8_t *py2 = (uint8_t*)(buf.map(0) + buf.stride(0) * (y + 1) + x);
+
+	uint8_t *puv = (uint8_t*)(buf.map(1) + buf.stride(1) * (y / 2) + x);
+
+	uint8_t y0 = yuv1.y;
+	uint8_t y1 = yuv2.y;
+	uint8_t y2 = yuv3.y;
+	uint8_t y3 = yuv4.y;
+	uint8_t u = (yuv1.u + yuv2.u + yuv3.u + yuv4.u) / 4;
+	uint8_t v = (yuv1.v + yuv2.v + yuv3.v + yuv4.v) / 4;
+
+	switch (buf.format()) {
+	case PixelFormat::NV12:
+		py1[0] = y0;
+		py1[1] = y1;
+		py2[0] = y2;
+		py2[1] = y3;
+		puv[0] = u;
+		puv[1] = v;
+		break;
+
+	case PixelFormat::NV21:
+		py1[0] = y0;
+		py1[1] = y1;
+		py2[0] = y2;
+		py2[1] = y3;
+		puv[0] = v;
+		puv[1] = u;
+		break;
+
+	default:
+		throw std::invalid_argument("invalid pixelformat");
+	}
+}
+
+static void draw_yuv420_planar_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
+					  YUV yuv1, YUV yuv2, YUV yuv3, YUV yuv4)
+{
+	uint8_t *py1 = (uint8_t*)(buf.map(0) + buf.stride(0) * (y + 0) + x);
+	uint8_t *py2 = (uint8_t*)(buf.map(0) + buf.stride(0) * (y + 1) + x);
+
+	uint8_t *pu = (uint8_t*)(buf.map(1) + buf.stride(1) * (y / 2) + x / 2);
+	uint8_t *pv = (uint8_t*)(buf.map(2) + buf.stride(2) * (y / 2) + x / 2);
+
+	uint8_t y0 = yuv1.y;
+	uint8_t y1 = yuv2.y;
+	uint8_t y2 = yuv3.y;
+	uint8_t y3 = yuv4.y;
+	uint8_t u = (yuv1.u + yuv2.u + yuv3.u + yuv4.u) / 4;
+	uint8_t v = (yuv1.v + yuv2.v + yuv3.v + yuv4.v) / 4;
+
+	switch (buf.format()) {
+	case PixelFormat::YUV420:
+		py1[0] = y0;
+		py1[1] = y1;
+		py2[0] = y2;
+		py2[1] = y3;
+		pu[0] = u;
+		pv[0] = v;
+		break;
+
+	case PixelFormat::YVU420:
+		py1[0] = y0;
+		py1[1] = y1;
+		py2[0] = y2;
+		py2[1] = y3;
+		pu[0] = v;
+		pv[0] = u;
+		break;
+
 	default:
 		throw std::invalid_argument("invalid pixelformat");
 	}
@@ -224,35 +367,15 @@ void draw_yuv420_macropixel(IFramebuffer& buf, unsigned x, unsigned y,
 	ASSERT((x & 1) == 0);
 	ASSERT((y & 1) == 0);
 
-	uint8_t *py1 = (uint8_t*)(buf.map(0) + buf.stride(0) * (y + 0) + x);
-	uint8_t *py2 = (uint8_t*)(buf.map(0) + buf.stride(0) * (y + 1) + x);
-
-	uint8_t *puv = (uint8_t*)(buf.map(1) + buf.stride(1) * (y / 2) + x);
-
-	uint8_t y0 = yuv1.y;
-	uint8_t y1 = yuv2.y;
-	uint8_t y2 = yuv3.y;
-	uint8_t y3 = yuv4.y;
-	uint8_t u = (yuv1.u + yuv2.u + yuv3.u + yuv4.u) / 4;
-	uint8_t v = (yuv1.v + yuv2.v + yuv3.v + yuv4.v) / 4;
-
 	switch (buf.format()) {
 	case PixelFormat::NV12:
-		py1[0] = y0;
-		py1[1] = y1;
-		py2[0] = y2;
-		py2[1] = y3;
-		puv[0] = u;
-		puv[1] = v;
-		break;
-
 	case PixelFormat::NV21:
-		py1[0] = y0;
-		py1[1] = y1;
-		py2[0] = y2;
-		py2[1] = y3;
-		puv[0] = v;
-		puv[1] = u;
+		draw_yuv420_semiplanar_macropixel(buf, x, y, yuv1, yuv2, yuv3, yuv4);
+		break;
+
+	case PixelFormat::YUV420:
+	case PixelFormat::YVU420:
+		draw_yuv420_planar_macropixel(buf, x, y, yuv1, yuv2, yuv3, yuv4);
 		break;
 
 	default:
@@ -283,12 +406,23 @@ void draw_rect(IFramebuffer &fb, uint32_t x, uint32_t y, uint32_t w, uint32_t h,
 		}
 		break;
 
+	case PixelFormat::YUV444:
+	case PixelFormat::YVU444:
+		for (j = 0; j < h; j++) {
+			for (i = 0; i < w; i++) {
+				draw_yuv444_pixel(fb, x + i, y + j, yuvcolor);
+			}
+		}
+		break;
+
 	case PixelFormat::UYVY:
 	case PixelFormat::YUYV:
 	case PixelFormat::YVYU:
 	case PixelFormat::VYUY:
 	case PixelFormat::NV16:
 	case PixelFormat::NV61:
+	case PixelFormat::YUV422:
+	case PixelFormat::YVU422:
 		for (j = 0; j < h; j++) {
 			for (i = 0; i < w; i += 2) {
 				draw_yuv422_macropixel(fb, x + i, y + j, yuvcolor, yuvcolor);
@@ -298,6 +432,8 @@ void draw_rect(IFramebuffer &fb, uint32_t x, uint32_t y, uint32_t w, uint32_t h,
 
 	case PixelFormat::NV12:
 	case PixelFormat::NV21:
+	case PixelFormat::YUV420:
+	case PixelFormat::YVU420:
 		for (j = 0; j < h; j += 2) {
 			for (i = 0; i < w; i += 2) {
 				draw_yuv420_macropixel(fb, x + i, y + j,
@@ -361,12 +497,25 @@ static void draw_char(IFramebuffer& buf, uint32_t xpos, uint32_t ypos, char c, R
 		}
 		break;
 
+	case PixelFormat::YUV444:
+	case PixelFormat::YVU444:
+		for (y = 0; y < 8; y++) {
+			for (x = 0; x < 8; x++) {
+				bool b = get_char_pixel(c, x, y);
+
+				draw_yuv444_pixel(buf, xpos + x, ypos + y, b ? yuvcolor : YUV(RGB()));
+			}
+		}
+		break;
+
 	case PixelFormat::UYVY:
 	case PixelFormat::YUYV:
 	case PixelFormat::YVYU:
 	case PixelFormat::VYUY:
 	case PixelFormat::NV16:
 	case PixelFormat::NV61:
+	case PixelFormat::YUV422:
+	case PixelFormat::YVU422:
 		for (y = 0; y < 8; y++) {
 			for (x = 0; x < 8; x += 2) {
 				bool b0 = get_char_pixel(c, x, y);
@@ -380,6 +529,8 @@ static void draw_char(IFramebuffer& buf, uint32_t xpos, uint32_t ypos, char c, R
 
 	case PixelFormat::NV12:
 	case PixelFormat::NV21:
+	case PixelFormat::YUV420:
+	case PixelFormat::YVU420:
 		for (y = 0; y < 8; y += 2) {
 			for (x = 0; x < 8; x += 2) {
 				bool b00 = get_char_pixel(c, x, y);
diff --git a/kms++util/src/testpat.cpp b/kms++util/src/testpat.cpp
index c120de34a101..ac386737b7cd 100644
--- a/kms++util/src/testpat.cpp
+++ b/kms++util/src/testpat.cpp
@@ -120,6 +120,15 @@ static void draw_test_pattern_part(IFramebuffer& fb, unsigned start_y, unsigned
 
 	case PixelColorType::YUV:
 		switch (plane_info.xsub + plane_info.ysub) {
+		case 2:
+			for (y = start_y; y < end_y; y++) {
+				for (x = 0; x < w; x++) {
+					RGB pixel = get_test_pattern_pixel(fb, x, y);
+					draw_yuv444_pixel(fb, x, y, pixel.yuv(yuvt));
+				}
+			}
+			break;
+
 		case 3:
 			for (y = start_y; y < end_y; y++) {
 				for (x = 0; x < w; x += 2) {
diff --git a/py/pykms/pykmsbase.cpp b/py/pykms/pykmsbase.cpp
index 003ad3c1a0b2..3e6defc88def 100644
--- a/py/pykms/pykmsbase.cpp
+++ b/py/pykms/pykmsbase.cpp
@@ -181,6 +181,13 @@ void init_pykmsbase(py::module &m)
 			.value("NV16", PixelFormat::NV16)
 			.value("NV61", PixelFormat::NV61)
 
+			.value("YUV420", PixelFormat::YUV420)
+			.value("YVU420", PixelFormat::YVU420)
+			.value("YUV422", PixelFormat::YUV422)
+			.value("YVU422", PixelFormat::YVU422)
+			.value("YUV444", PixelFormat::YUV444)
+			.value("YVU444", PixelFormat::YVU444)
+
 			.value("UYVY", PixelFormat::UYVY)
 			.value("YUYV", PixelFormat::YUYV)
 			.value("YVYU", PixelFormat::YVYU)
-- 
Regards,

Laurent Pinchart

[PATCH 8/8] kms++: Add support for missing 8 -and 16-bit RGB formats

[Laurent Pinchart]

Add support for the RGB332, XRGB1555 and XRGB4444 formats to the
PixelFormat class, the Python API, and the drawing utilities.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 kms++/inc/kms++/pixelformats.h  |  5 +++++
 kms++/src/pixelformats.cpp      |  9 ++++++---
 kms++util/inc/kms++util/color.h |  1 +
 kms++util/src/color.cpp         |  5 +++++
 kms++util/src/drawing.cpp       | 14 ++++++++++++++
 py/pykms/pykmsbase.cpp          |  5 +++++
 6 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/kms++/inc/kms++/pixelformats.h b/kms++/inc/kms++/pixelformats.h
index 746f9e2b8c5a..4e453a2f594d 100644
--- a/kms++/inc/kms++/pixelformats.h
+++ b/kms++/inc/kms++/pixelformats.h
@@ -45,9 +45,14 @@ enum class PixelFormat : uint32_t
 	RGB888 = MakeFourCC("RG24"),
 	BGR888 = MakeFourCC("BG24"),
 
+	RGB332 = MakeFourCC("RGB8"),
+
 	RGB565 = MakeFourCC("RG16"),
 	BGR565 = MakeFourCC("BG16"),
 
+	XRGB4444 = MakeFourCC("XR12"),
+	XRGB1555 = MakeFourCC("XR15"),
+
 	ARGB4444 = MakeFourCC("AR12"),
 	ARGB1555 = MakeFourCC("AR15"),
 
diff --git a/kms++/src/pixelformats.cpp b/kms++/src/pixelformats.cpp
index fe86fca04cd8..2c60c1e673f3 100644
--- a/kms++/src/pixelformats.cpp
+++ b/kms++/src/pixelformats.cpp
@@ -24,9 +24,15 @@ static const map<PixelFormat, PixelFormatInfo> format_info_array = {
 	{ PixelFormat::YVU422, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 2, 1 }, { 8, 2, 1 } }, } },
 	{ PixelFormat::YUV444, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 1, 1 }, { 8, 1, 1 } }, } },
 	{ PixelFormat::YVU444, { PixelColorType::YUV, 3, { { 8, 1, 1, }, { 8, 1, 1 }, { 8, 1, 1 } }, } },
+	/* RGB8 */
+	{ PixelFormat::RGB332, { PixelColorType::RGB, 1, { { 8, 1, 1 } }, } },
 	/* RGB16 */
 	{ PixelFormat::RGB565, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
 	{ PixelFormat::BGR565, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
+	{ PixelFormat::XRGB4444, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
+	{ PixelFormat::XRGB1555, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
+	{ PixelFormat::ARGB4444, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
+	{ PixelFormat::ARGB1555, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
 	/* RGB24 */
 	{ PixelFormat::RGB888, { PixelColorType::RGB, 1, { { 24, 1, 1 } }, } },
 	{ PixelFormat::BGR888, { PixelColorType::RGB, 1, { { 24, 1, 1 } }, } },
@@ -50,9 +56,6 @@ static const map<PixelFormat, PixelFormatInfo> format_info_array = {
 	{ PixelFormat::ABGR2101010, { PixelColorType::RGB, 1, { { 32, 1, 1 } }, } },
 	{ PixelFormat::RGBA1010102, { PixelColorType::RGB, 1, { { 32, 1, 1 } }, } },
 	{ PixelFormat::BGRA1010102, { PixelColorType::RGB, 1, { { 32, 1, 1 } }, } },
-
-	{ PixelFormat::ARGB4444, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
-	{ PixelFormat::ARGB1555, { PixelColorType::RGB, 1, { { 16, 1, 1 } }, } },
 };
 
 const struct PixelFormatInfo& get_pixel_format_info(PixelFormat format)
diff --git a/kms++util/inc/kms++util/color.h b/kms++util/inc/kms++util/color.h
index 2bf6e66315a4..fa05fbcc1982 100644
--- a/kms++util/inc/kms++util/color.h
+++ b/kms++util/inc/kms++util/color.h
@@ -34,6 +34,7 @@ struct RGB
 	uint32_t rgba1010102() const;
 	uint32_t bgra1010102() const;
 
+	uint8_t rgb332() const;
 	uint16_t rgb565() const;
 	uint16_t bgr565() const;
 	uint16_t argb4444() const;
diff --git a/kms++util/src/color.cpp b/kms++util/src/color.cpp
index 80e486668298..2d45eff5d70b 100644
--- a/kms++util/src/color.cpp
+++ b/kms++util/src/color.cpp
@@ -79,6 +79,11 @@ uint32_t RGB::bgra1010102() const
 	return (b << 24) | (g << 14) | (r << 4) | (a >> 6);
 }
 
+uint8_t RGB::rgb332() const
+{
+	return ((r >> 5) << 5) | ((g >> 5) << 2) | ((b >> 6) << 0);
+}
+
 uint16_t RGB::rgb565() const
 {
 	return ((r >> 3) << 11) | ((g >> 2) << 5) | ((b >> 3) << 0);
diff --git a/kms++util/src/drawing.cpp b/kms++util/src/drawing.cpp
index c1c639a8b3a5..3752f94695e0 100644
--- a/kms++util/src/drawing.cpp
+++ b/kms++util/src/drawing.cpp
@@ -86,6 +86,12 @@ void draw_rgb_pixel(IFramebuffer& buf, unsigned x, unsigned y, RGB color)
 		p[2] = color.b;
 		break;
 	}
+	case PixelFormat::RGB332:
+	{
+		uint8_t *p = (uint8_t*)(buf.map(0) + buf.stride(0) * y + x);
+		*p = color.rgb332();
+		break;
+	}
 	case PixelFormat::RGB565:
 	{
 		uint16_t *p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * 2);
@@ -98,12 +104,14 @@ void draw_rgb_pixel(IFramebuffer& buf, unsigned x, unsigned y, RGB color)
 		*p = color.bgr565();
 		break;
 	}
+	case PixelFormat::XRGB4444:
 	case PixelFormat::ARGB4444:
 	{
 		uint16_t *p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * 2);
 		*p = color.argb4444();
 		break;
 	}
+	case PixelFormat::XRGB1555:
 	case PixelFormat::ARGB1555:
 	{
 		uint16_t *p = (uint16_t*)(buf.map(0) + buf.stride(0) * y + x * 2);
@@ -397,8 +405,11 @@ void draw_rect(IFramebuffer &fb, uint32_t x, uint32_t y, uint32_t w, uint32_t h,
 	case PixelFormat::BGR888:
 	case PixelFormat::RGB565:
 	case PixelFormat::BGR565:
+	case PixelFormat::XRGB4444:
+	case PixelFormat::XRGB1555:
 	case PixelFormat::ARGB4444:
 	case PixelFormat::ARGB1555:
+	case PixelFormat::RGB332:
 		for (j = 0; j < h; j++) {
 			for (i = 0; i < w; i++) {
 				draw_rgb_pixel(fb, x + i, y + j, color);
@@ -486,8 +497,11 @@ static void draw_char(IFramebuffer& buf, uint32_t xpos, uint32_t ypos, char c, R
 	case PixelFormat::BGR888:
 	case PixelFormat::RGB565:
 	case PixelFormat::BGR565:
+	case PixelFormat::XRGB4444:
+	case PixelFormat::XRGB1555:
 	case PixelFormat::ARGB4444:
 	case PixelFormat::ARGB1555:
+	case PixelFormat::RGB332:
 		for (y = 0; y < 8; y++) {
 			for (x = 0; x < 8; x++) {
 				bool b = get_char_pixel(c, x, y);
diff --git a/py/pykms/pykmsbase.cpp b/py/pykms/pykmsbase.cpp
index 3e6defc88def..0448e0264ded 100644
--- a/py/pykms/pykmsbase.cpp
+++ b/py/pykms/pykmsbase.cpp
@@ -206,9 +206,14 @@ void init_pykmsbase(py::module &m)
 			.value("RGB888", PixelFormat::RGB888)
 			.value("BGR888", PixelFormat::BGR888)
 
+			.value("RGB332", PixelFormat::RGB332)
+
 			.value("RGB565", PixelFormat::RGB565)
 			.value("BGR565", PixelFormat::BGR565)
 
+			.value("XRGB4444", PixelFormat::XRGB4444)
+			.value("XRGB1555", PixelFormat::XRGB1555)
+
 			.value("ARGB4444", PixelFormat::ARGB4444)
 			.value("ARGB1555", PixelFormat::ARGB1555)
 
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/8] card: Add a method to retrieve the device minor

[Tomi Valkeinen]

On 06/08/2020 05:17, Laurent Pinchart wrote:
> The device minor number is needed to access the debugfs directory
> corresponding to the device. Make it available to users through a
> get_minor() method on the Card object.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  kms++/inc/kms++/card.h |  3 +++
>  kms++/src/card.cpp     | 11 +++++++++++
>  py/pykms/pykmsbase.cpp |  1 +
>  3 files changed, 15 insertions(+)
> 
> diff --git a/kms++/inc/kms++/card.h b/kms++/inc/kms++/card.h
> index 5c1cf7cfcedc..0a11747f7985 100644
> --- a/kms++/inc/kms++/card.h
> +++ b/kms++/inc/kms++/card.h
> @@ -35,6 +35,7 @@ public:
>  	Card& operator=(const Card& other) = delete;
>  
>  	int fd() const { return m_fd; }
> +	unsigned int dev_minor() const { return m_minor; }
>  
>  	void drop_master();
>  
> @@ -84,7 +85,9 @@ private:
>  	std::vector<Framebuffer*> m_framebuffers;
>  
>  	int m_fd;
> +	unsigned int m_minor;
>  	bool m_is_master;
> +	std::string m_device;

This looks like an extra change.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 4/8] dumbfb: Fix pitch for tri-planar formats

[Tomi Valkeinen]

On 06/08/2020 05:18, Laurent Pinchart wrote:
> The BO pitches are unconditionally set to the frame buffer pitch, for
> all planes. This is correct for semiplanar YUV formats, as they
> subsample chroma horizontally by two but combined U and V in a single
> plane, cancelling each other. For fully planar YUV formats, however, the
> horizontal subsampling need to be taken into account to compute the
> pitch. Fix it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  kms++/src/dumbframebuffer.cpp | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/kms++/src/dumbframebuffer.cpp b/kms++/src/dumbframebuffer.cpp
> index 18f3f152943d..4c3c03164a90 100644
> --- a/kms++/src/dumbframebuffer.cpp
> +++ b/kms++/src/dumbframebuffer.cpp
> @@ -42,6 +42,14 @@ DumbFramebuffer::DumbFramebuffer(Card& card, uint32_t width, uint32_t height, Pi
>  		struct drm_mode_create_dumb creq = drm_mode_create_dumb();
>  		creq.width = width;
>  		creq.height = height / pi.ysub;
> +		/*
> +		 * For fully planar YUV buffers, the chroma planes don't combine
> +		 * U and V components, their width must thus be divided by the
> +		 * horizontal subsampling factor.
> +		 */
> +		if (format_info.type == PixelColorType::YUV &&
> +		    format_info.num_planes == 3)
> +			creq.width /= pi.xsub;

This feels a bit of a hack... I think we should somehow have the relevant information in the
PixelFormatInfo. Having the same plane info, { 8, 2, 2 }, for both NV12 UV plane and YUV420 U and V
planes doesn't sound correct.

Should NV12's UV plane be { 16, 2, 2 }? Subsampled formats are confusing... =)

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 0/8] kmsxx: Various fixes and improvements

[Tomi Valkeinen]

On 06/08/2020 05:17, Laurent Pinchart wrote:
> Hi Tomi,
> 
> This patch series fixes a few issues in kmsxx, mainly related to support
> for additional pixel formats.

Thanks, looks good aside the two comments. And for the one for pixel formats, I think we can go with
what you have, and try to figure out a clean solution later (unless you already have an idea).

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/8] card: Add a method to retrieve the device minor

[Sergei Shtylyov]

Hello!

On 06.08.2020 5:17, Laurent Pinchart wrote:

> The device minor number is needed to access the debugfs directory
> corresponding to the device. Make it available to users through a
> get_minor() method on the Card object.

    Didn't you call it dev_minor() below?

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
[...]
> diff --git a/kms++/inc/kms++/card.h b/kms++/inc/kms++/card.h
> index 5c1cf7cfcedc..0a11747f7985 100644
> --- a/kms++/inc/kms++/card.h
> +++ b/kms++/inc/kms++/card.h
> @@ -35,6 +35,7 @@ public:
>   	Card& operator=(const Card& other) = delete;
>   
>   	int fd() const { return m_fd; }
> +	unsigned int dev_minor() const { return m_minor; }
>   
>   	void drop_master();
>   
> @@ -84,7 +85,9 @@ private:
>   	std::vector<Framebuffer*> m_framebuffers;
>   
>   	int m_fd;
> +	unsigned int m_minor;
>   	bool m_is_master;
> +	std::string m_device;
>   
>   	bool m_has_atomic;
>   	bool m_has_universal_planes;
[...]

MBR, Sergei

Re: [PATCH 4/8] dumbfb: Fix pitch for tri-planar formats

[Laurent Pinchart]

Hi Tomi,

On Thu, Aug 06, 2020 at 12:21:39PM +0300, Tomi Valkeinen wrote:
> On 06/08/2020 05:18, Laurent Pinchart wrote:
> > The BO pitches are unconditionally set to the frame buffer pitch, for
> > all planes. This is correct for semiplanar YUV formats, as they
> > subsample chroma horizontally by two but combined U and V in a single
> > plane, cancelling each other. For fully planar YUV formats, however, the
> > horizontal subsampling need to be taken into account to compute the
> > pitch. Fix it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  kms++/src/dumbframebuffer.cpp | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/kms++/src/dumbframebuffer.cpp b/kms++/src/dumbframebuffer.cpp
> > index 18f3f152943d..4c3c03164a90 100644
> > --- a/kms++/src/dumbframebuffer.cpp
> > +++ b/kms++/src/dumbframebuffer.cpp
> > @@ -42,6 +42,14 @@ DumbFramebuffer::DumbFramebuffer(Card& card, uint32_t width, uint32_t height, Pi
> >  		struct drm_mode_create_dumb creq = drm_mode_create_dumb();
> >  		creq.width = width;
> >  		creq.height = height / pi.ysub;
> > +		/*
> > +		 * For fully planar YUV buffers, the chroma planes don't combine
> > +		 * U and V components, their width must thus be divided by the
> > +		 * horizontal subsampling factor.
> > +		 */
> > +		if (format_info.type == PixelColorType::YUV &&
> > +		    format_info.num_planes == 3)
> > +			creq.width /= pi.xsub;
> 
> This feels a bit of a hack... I think we should somehow have the
> relevant information in the PixelFormatInfo. Having the same plane
> info, { 8, 2, 2 }, for both NV12 UV plane and YUV420 U and V planes
> doesn't sound correct.
> 
> Should NV12's UV plane be { 16, 2, 2 }? Subsampled formats are
> confusing... =)

I'll give it a try. I however wonder if all drivers will expect 16bpp.
The ones based on drm_gem_cma_dumb_create() will be fine, but other
drivers may not expect this.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/8] card: Add a method to retrieve the device minor

[Laurent Pinchart]

Hi Tomi,

On Thu, Aug 06, 2020 at 10:46:43AM +0300, Tomi Valkeinen wrote:
> On 06/08/2020 05:17, Laurent Pinchart wrote:
> > The device minor number is needed to access the debugfs directory
> > corresponding to the device. Make it available to users through a
> > get_minor() method on the Card object.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  kms++/inc/kms++/card.h |  3 +++
> >  kms++/src/card.cpp     | 11 +++++++++++
> >  py/pykms/pykmsbase.cpp |  1 +
> >  3 files changed, 15 insertions(+)
> > 
> > diff --git a/kms++/inc/kms++/card.h b/kms++/inc/kms++/card.h
> > index 5c1cf7cfcedc..0a11747f7985 100644
> > --- a/kms++/inc/kms++/card.h
> > +++ b/kms++/inc/kms++/card.h
> > @@ -35,6 +35,7 @@ public:
> >  	Card& operator=(const Card& other) = delete;
> >  
> >  	int fd() const { return m_fd; }
> > +	unsigned int dev_minor() const { return m_minor; }
> >  
> >  	void drop_master();
> >  
> > @@ -84,7 +85,9 @@ private:
> >  	std::vector<Framebuffer*> m_framebuffers;
> >  
> >  	int m_fd;
> > +	unsigned int m_minor;
> >  	bool m_is_master;
> > +	std::string m_device;
> 
> This looks like an extra change.

Oops, indeed.

Should I submit a v2 of the whole series to address your other concerns,
or do you plan to already merge some of the patches ? In the latter
case, feel free to give this small issue when applying :-) (along with
s/get_minor/dev_minor/ in the commit message as pointed our by Sergei).

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/8] dumbfb: Fix pitch for tri-planar formats

[Tomi Valkeinen]

On 09/08/2020 01:14, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Thu, Aug 06, 2020 at 12:21:39PM +0300, Tomi Valkeinen wrote:
>> On 06/08/2020 05:18, Laurent Pinchart wrote:
>>> The BO pitches are unconditionally set to the frame buffer pitch, for
>>> all planes. This is correct for semiplanar YUV formats, as they
>>> subsample chroma horizontally by two but combined U and V in a single
>>> plane, cancelling each other. For fully planar YUV formats, however, the
>>> horizontal subsampling need to be taken into account to compute the
>>> pitch. Fix it.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>>  kms++/src/dumbframebuffer.cpp | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/kms++/src/dumbframebuffer.cpp b/kms++/src/dumbframebuffer.cpp
>>> index 18f3f152943d..4c3c03164a90 100644
>>> --- a/kms++/src/dumbframebuffer.cpp
>>> +++ b/kms++/src/dumbframebuffer.cpp
>>> @@ -42,6 +42,14 @@ DumbFramebuffer::DumbFramebuffer(Card& card, uint32_t width, uint32_t height, Pi
>>>  		struct drm_mode_create_dumb creq = drm_mode_create_dumb();
>>>  		creq.width = width;
>>>  		creq.height = height / pi.ysub;
>>> +		/*
>>> +		 * For fully planar YUV buffers, the chroma planes don't combine
>>> +		 * U and V components, their width must thus be divided by the
>>> +		 * horizontal subsampling factor.
>>> +		 */
>>> +		if (format_info.type == PixelColorType::YUV &&
>>> +		    format_info.num_planes == 3)
>>> +			creq.width /= pi.xsub;
>>
>> This feels a bit of a hack... I think we should somehow have the
>> relevant information in the PixelFormatInfo. Having the same plane
>> info, { 8, 2, 2 }, for both NV12 UV plane and YUV420 U and V planes
>> doesn't sound correct.
>>
>> Should NV12's UV plane be { 16, 2, 2 }? Subsampled formats are
>> confusing... =)
> 
> I'll give it a try. I however wonder if all drivers will expect 16bpp.
> The ones based on drm_gem_cma_dumb_create() will be fine, but other
> drivers may not expect this.

Oh, right, that number is passed to DRM_IOCTL_MODE_CREATE_DUMB. I was only thinking about how kms++
handles these internally.

To be honest, I don't even quite know how subsampled formats are supposed to be handled in DRM.
Above we pass width as it is, but divide height by ysub. And then we tune the bpp adjust to the fact
that we didn't divide the width.

For e.g. YUYV, the bpp tells the container size. But for NV12's second plane, bpp is not the
container size.

Well, I think at least omapdrm doesn't care, it just allocates the number of bytes according to
w*h*bpp. But maybe it would be good to have a clear rule how to represent these in kms++, and then
if DRM wants the values in some other way, adjust the values according to the format.

Or maybe we already have all the numbers according to a clear rule, I'm just not sure what the rule
is =).

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 4/8] dumbfb: Fix pitch for tri-planar formats

[Tomi Valkeinen]

On 10/08/2020 09:16, Tomi Valkeinen wrote:

> To be honest, I don't even quite know how subsampled formats are supposed to be handled in DRM.
> Above we pass width as it is, but divide height by ysub. And then we tune the bpp adjust to the fact
> that we didn't divide the width.
> 
> For e.g. YUYV, the bpp tells the container size. But for NV12's second plane, bpp is not the
> container size.

Ah, no it isn't... Not enough coffee yet.

So the bpp for YUYV and NV12 is the "effective bpp", how many bits are consumed on that plane for
each real pixel. Then, shouldn't bpp for YUV420's second and third plane be 4?

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/8] card: Add a method to retrieve the device minor

[Tomi Valkeinen]

On 10/08/2020 08:54, Laurent Pinchart wrote:

> Should I submit a v2 of the whole series to address your other concerns,
> or do you plan to already merge some of the patches ? In the latter
> case, feel free to give this small issue when applying :-) (along with
> s/get_minor/dev_minor/ in the commit message as pointed our by Sergei).

I fixed these, and pushed the series. We can ponder about the YUV bpps on top.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki