[PATCH v1 0/4] Add MacroSilicon MS91xx USB display driver

[PATCH v1 0/4] Add MacroSilicon MS91xx USB display driver

[Ho Jie Feng]

This series adds a DRM/KMS driver for MacroSilicon MS912x/MS913x
USB display adapters.

The four patches add:
  1. XRGB8888-to-UYVY conversion helper.
  2. Register access and connector support.
  3. Atomic modesetting and USB framebuffer transfers.
  4. Kconfig, Makefile and MAINTAINERS integration.

Some code is referenced from the vendor linux driver.

Tested on USB to Composite/VGA/HDMI adapters, both USB2 and USB3.

Ho Jie Feng (4):
  drm/format-helper: Add XRGB8888 to UYVY conversion
  drm/ms91xx: Add register access and connector support
  drm/ms91xx: Add atomic modesetting and USB framebuffer transfers
  drm/ms91xx: Add Kconfig and Makefile integration

 MAINTAINERS                               |   7 +
 drivers/gpu/drm/Kconfig                   |   1 +
 drivers/gpu/drm/Makefile                  |   1 +
 drivers/gpu/drm/drm_format_helper.c       |  52 +++
 drivers/gpu/drm/drm_format_internal.h     |  19 +
 drivers/gpu/drm/ms91xx/Kconfig            |  13 +
 drivers/gpu/drm/ms91xx/Makefile           |   9 +
 drivers/gpu/drm/ms91xx/ms91xx.h           | 217 +++++++++
 drivers/gpu/drm/ms91xx/ms91xx_connector.c | 263 +++++++++++
 drivers/gpu/drm/ms91xx/ms91xx_drv.c       | 518 ++++++++++++++++++++++
 drivers/gpu/drm/ms91xx/ms91xx_registers.c | 386 ++++++++++++++++
 drivers/gpu/drm/ms91xx/ms91xx_transfer.c  | 218 +++++++++
 include/drm/drm_format_helper.h           |   4 +
 13 files changed, 1708 insertions(+)
 create mode 100644 drivers/gpu/drm/ms91xx/Kconfig
 create mode 100644 drivers/gpu/drm/ms91xx/Makefile
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx.h
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_connector.c
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_drv.c
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_registers.c
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_transfer.c


base-commit: 5e756743945d3b6bde6d8d1e3f943a03269c0ece
-- 
2.55.0

[PATCH v1 1/4] drm/format-helper: Add XRGB8888 to UYVY conversion

[Ho Jie Feng]

Add a helper for devices that use packed UYVY but accept XRGB8888
framebuffers.

Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/gpu/drm/drm_format_helper.c   | 52 +++++++++++++++++++++++++++
 drivers/gpu/drm/drm_format_internal.h | 19 ++++++++++
 include/drm/drm_format_helper.h       |  4 +++
 3 files changed, 75 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 6cddf05c493bd..3cdfb4c68bfbf 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -11,6 +11,7 @@
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/iosys-map.h>
+#include <linux/math.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 
@@ -1379,3 +1380,54 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray2);
 
+static void drm_fb_xrgb8888_to_uyvy_line(void *dbuf, const void *sbuf, unsigned int pixels)
+{
+	__le32 *dbuf32 = dbuf;
+	const __le32 *sbuf32 = sbuf;
+	const __le32 *send32 = sbuf32 + round_down(pixels, 2);
+
+	while (sbuf32 < send32) {
+		u32 pix1 = le32_to_cpup(sbuf32);
+		u32 pix2 = le32_to_cpup(sbuf32 + 1);
+		u32 val32 = drm_pixel_xrgb8888_to_uyvy_bt601(pix1, pix2);
+
+		*dbuf32++ = cpu_to_le32(val32);
+		sbuf32 += 2;
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_uyvy - Convert XRGB8888 to UYVY clip buffer
+ * @dst: Array of UYVY destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffers
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy; x1 and x2 must be even
+ * @state: Transform and conversion state
+ *
+ * This function copies parts of a framebuffer to display memory and converts the
+ * color format during the process. The parameters @dst, @dst_pitch and @src refer
+ * to arrays. Each array must have at least as many entries as there are planes in
+ * @fb's format. Each entry stores the value for the format's respective color plane
+ * at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for UYVY devices that don't support XRGB8888
+ * natively. ITU BT.601 is being used for the RGB -> YUV conversion.
+ */
+void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
+			     const struct iosys_map *src, const struct drm_framebuffer *fb,
+			     const struct drm_rect *clip, struct drm_format_conv_state *state)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		2,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, state,
+		    drm_fb_xrgb8888_to_uyvy_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_uyvy);
+
diff --git a/drivers/gpu/drm/drm_format_internal.h b/drivers/gpu/drm/drm_format_internal.h
index ce29dd05bcc55..41875656e59e4 100644
--- a/drivers/gpu/drm/drm_format_internal.h
+++ b/drivers/gpu/drm/drm_format_internal.h
@@ -159,6 +159,25 @@ static inline u32 drm_pixel_xrgb8888_to_abgr2101010(u32 pix)
 	       drm_pixel_xrgb8888_to_xbgr2101010(pix);
 }
 
+static inline u32 drm_pixel_xrgb8888_to_uyvy_bt601(u32 pix1, u32 pix2)
+{
+	u32 r1 = (pix1 & 0x00ff0000) >> 16;
+	u32 g1 = (pix1 & 0x0000ff00) >> 8;
+	u32 b1 =  pix1 & 0x000000ff;
+	u32 r2 = (pix2 & 0x00ff0000) >> 16;
+	u32 g2 = (pix2 & 0x0000ff00) >> 8;
+	u32 b2 =  pix2 & 0x000000ff;
+	u32 r = r1 + r2;
+	u32 g = g1 + g2;
+	u32 b = b1 + b2;
+	u32 y1 = ((16U << 16) + 16829 * r1 + 33039 * g1 + 6416 * b1 + 32768) >> 16;
+	u32 y2 = ((16U << 16) + 16829 * r2 + 33039 * g2 + 6416 * b2 + 32768) >> 16;
+	u32 u = ((128U << 17) - 9714 * r - 19071 * g + 28784 * b + 65536) >> 17;
+	u32 v = ((128U << 17) + 28784 * r - 24103 * g - 4681 * b + 65536) >> 17;
+
+	return u | (y1 << 8) | (v << 16) | (y2 << 24);
+}
+
 /*
  * Conversion from ARGB8888
  */
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 2b5c1aef80b0e..e50f96e07484c 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -136,4 +136,8 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
 			      const struct iosys_map *src, const struct drm_framebuffer *fb,
 			      const struct drm_rect *clip, struct drm_format_conv_state *state);
 
+void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
+			     const struct iosys_map *src, const struct drm_framebuffer *fb,
+			     const struct drm_rect *clip, struct drm_format_conv_state *state);
+
 #endif /* __LINUX_DRM_FORMAT_HELPER_H */
-- 
2.55.0

[PATCH v1 2/4] drm/ms91xx: Add register access and connector support

[Ho Jie Feng]

Implement for the ms91xx driver:
- USB control requests for register and flash access
- display power
- mode programming
- custom timings stored in device flash
- connector detection
- EDID mode
- fallback modes

Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/gpu/drm/ms91xx/ms91xx_connector.c | 263 +++++++++++++++
 drivers/gpu/drm/ms91xx/ms91xx_registers.c | 386 ++++++++++++++++++++++
 2 files changed, 649 insertions(+)
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_connector.c
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_registers.c

diff --git a/drivers/gpu/drm/ms91xx/ms91xx_connector.c b/drivers/gpu/drm/ms91xx/ms91xx_connector.c
new file mode 100644
index 0000000000000..d04f818fa09f2
--- /dev/null
+++ b/drivers/gpu/drm/ms91xx/ms91xx_connector.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/array_size.h>
+#include <linux/types.h>
+
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_device.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_mode_config.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_print.h>
+
+#include "ms91xx.h"
+
+static int ms91xx_get_connector_type(struct ms91xx_device *ms91xx)
+{
+	int port_type;
+
+	port_type = ms91xx_read_byte(ms91xx, MS91XX_REG_VIDEO_PORT);
+	if (port_type < 0) {
+		drm_warn(&ms91xx->drm,
+			 "failed to read video port type: %d\n", port_type);
+		ms91xx->port_type = MS91XX_VIDEO_PORT_UNKNOWN;
+		return DRM_MODE_CONNECTOR_Unknown;
+	}
+
+	ms91xx->port_type = port_type;
+
+	switch (ms91xx->port_type) {
+	case MS91XX_VIDEO_PORT_CVBS:
+		return DRM_MODE_CONNECTOR_Composite;
+	case MS91XX_VIDEO_PORT_VGA:
+		return DRM_MODE_CONNECTOR_VGA;
+	case MS91XX_VIDEO_PORT_YPBPR:
+		return DRM_MODE_CONNECTOR_Component;
+	/*
+	 * Combo VGA+HDMI device seems to report this as connector type
+	 * Since there doesn't seem to be any S-Video devices out there,
+	 * set this to HDMI instead.
+	 */
+	case MS91XX_VIDEO_PORT_SVIDEO:
+	case MS91XX_VIDEO_PORT_HDMI:
+		return DRM_MODE_CONNECTOR_HDMIA;
+	case MS91XX_VIDEO_PORT_DIGITAL:
+		return DRM_MODE_CONNECTOR_DPI;
+	case MS91XX_VIDEO_PORT_CVBS_SVIDEO:
+		return DRM_MODE_CONNECTOR_9PinDIN;
+	default:
+		drm_warn(&ms91xx->drm,
+			 "unknown video port type: %d\n", port_type);
+		ms91xx->port_type = MS91XX_VIDEO_PORT_UNKNOWN;
+		return DRM_MODE_CONNECTOR_Unknown;
+	}
+}
+
+static int
+ms91xx_get_encoder_type(enum ms91xx_video_port port_type)
+{
+	switch (port_type) {
+	case MS91XX_VIDEO_PORT_VGA:
+		return DRM_MODE_ENCODER_DAC;
+	case MS91XX_VIDEO_PORT_CVBS:
+	case MS91XX_VIDEO_PORT_YPBPR:
+	case MS91XX_VIDEO_PORT_CVBS_SVIDEO:
+		return DRM_MODE_ENCODER_TVDAC;
+	/* See above comment */
+	case MS91XX_VIDEO_PORT_SVIDEO:
+	case MS91XX_VIDEO_PORT_HDMI:
+		return DRM_MODE_ENCODER_TMDS;
+	case MS91XX_VIDEO_PORT_DIGITAL:
+		return DRM_MODE_ENCODER_DPI;
+	case MS91XX_VIDEO_PORT_UNKNOWN:
+	default:
+		return DRM_MODE_ENCODER_NONE;
+	}
+}
+
+static int ms91xx_read_edid(void *data, u8 *buf, unsigned int block, size_t len)
+{
+	struct ms91xx_device *ms91xx = data;
+	int offset = block * EDID_LENGTH;
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < len; i++) {
+		u16 address = MS91XX_REG_EDID_BASE + offset + i;
+
+		ret = ms91xx_read_byte(ms91xx, address);
+		if (ret < 0)
+			return ret;
+		buf[i] = ret;
+	}
+	return 0;
+}
+
+static int ms91xx_add_cea_modes(struct drm_connector *connector,
+				const u8 *vics, size_t num_vics)
+{
+	struct drm_display_mode *mode;
+	unsigned int i;
+	int count = 0;
+
+	drm_edid_connector_update(connector, NULL);
+
+	for (i = 0; i < num_vics; i++) {
+		mode = drm_display_mode_from_cea_vic(connector->dev, vics[i]);
+		if (!mode)
+			continue;
+
+		if (!count)
+			mode->type |= DRM_MODE_TYPE_PREFERRED;
+		drm_mode_probed_add(connector, mode);
+		count++;
+	}
+
+	return count;
+}
+
+static int ms91xx_add_cvbs_svideo_modes(struct drm_connector *connector)
+{
+	static const u8 vics[] = {
+		2,  /* 720x480p60 */
+		17, /* 720x576p50 */
+	};
+
+	return ms91xx_add_cea_modes(connector, vics, ARRAY_SIZE(vics));
+}
+
+static int ms91xx_add_ypbpr_modes(struct drm_connector *connector)
+{
+	static const u8 vics[] = {
+		4,  /* 1280x720p60 */
+		16, /* 1920x1080p60 */
+		2,  /* 720x480p60 */
+		17, /* 720x576p50 */
+	};
+
+	return ms91xx_add_cea_modes(connector, vics, ARRAY_SIZE(vics));
+}
+
+static int
+ms91xx_add_default_hdmi_vga_modes(struct drm_connector *connector)
+{
+	struct drm_mode_config *mode_config = &connector->dev->mode_config;
+	int count;
+
+	drm_edid_connector_update(connector, NULL);
+	count = drm_add_modes_noedid(connector, mode_config->max_width,
+				     mode_config->max_height);
+	if (count)
+		drm_set_preferred_mode(connector, 1024, 768);
+
+	return count;
+}
+
+static int ms91xx_add_custom_mode(struct drm_connector *connector)
+{
+	struct ms91xx_device *ms91xx = to_ms91xx(connector->dev);
+	struct drm_display_mode *mode;
+	unsigned int i;
+	int count = 0;
+
+	for (i = 0; i < ms91xx->num_custom_modes; i++) {
+		mode = drm_mode_duplicate(
+			connector->dev,
+			&ms91xx->custom_modes[i].display_mode);
+		if (!mode)
+			continue;
+
+		drm_mode_probed_add(connector, mode);
+		count++;
+	}
+
+	return count;
+}
+
+static int ms91xx_connector_get_modes(struct drm_connector *connector)
+{
+	struct ms91xx_device *ms91xx = to_ms91xx(connector->dev);
+	const struct drm_edid *edid;
+	int ret = ms91xx_add_custom_mode(connector);
+
+	if (ms91xx->port_type == MS91XX_VIDEO_PORT_CVBS ||
+	    ms91xx->port_type == MS91XX_VIDEO_PORT_CVBS_SVIDEO ||
+	    ms91xx->port_type == MS91XX_VIDEO_PORT_UNKNOWN)
+		return ret + ms91xx_add_cvbs_svideo_modes(connector);
+
+	if (ms91xx->port_type == MS91XX_VIDEO_PORT_YPBPR)
+		return ret + ms91xx_add_ypbpr_modes(connector);
+
+	edid = drm_edid_read_custom(connector, ms91xx_read_edid, ms91xx);
+	if (!edid)
+		return ret + ms91xx_add_default_hdmi_vga_modes(connector);
+
+	if (drm_edid_connector_update(connector, edid) >= 0)
+		ret += drm_edid_connector_add_modes(connector);
+	drm_edid_free(edid);
+	return ret;
+}
+
+static enum drm_connector_status ms91xx_detect(struct drm_connector *connector,
+					       bool force)
+{
+	struct ms91xx_device *ms91xx = to_ms91xx(connector->dev);
+	int status = ms91xx_read_byte(ms91xx, MS91XX_REG_DISPLAY_STATUS);
+
+	if (status < 0)
+		return connector_status_unknown;
+
+	return status == 1 ? connector_status_connected :
+			     connector_status_disconnected;
+}
+
+static const struct drm_encoder_funcs ms91xx_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static const struct drm_connector_helper_funcs ms91xx_connector_helper_funcs = {
+	.get_modes = ms91xx_connector_get_modes,
+};
+
+static const struct drm_connector_funcs ms91xx_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.detect = ms91xx_detect,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+int ms91xx_connector_init(struct ms91xx_device *ms91xx)
+{
+	int ret;
+	int connector_type;
+	int encoder_type;
+
+	connector_type = ms91xx_get_connector_type(ms91xx);
+	encoder_type = ms91xx_get_encoder_type(ms91xx->port_type);
+
+	ret = drm_encoder_init(&ms91xx->drm, &ms91xx->encoder,
+			       &ms91xx_encoder_funcs, encoder_type, NULL);
+	if (ret)
+		return ret;
+	ms91xx->encoder.possible_crtcs = drm_crtc_mask(&ms91xx->crtc);
+
+	drm_connector_helper_add(&ms91xx->connector,
+				 &ms91xx_connector_helper_funcs);
+	ret = drm_connector_init(&ms91xx->drm, &ms91xx->connector,
+				 &ms91xx_connector_funcs, connector_type);
+	if (ret)
+		return ret;
+
+	ms91xx->connector.polled =
+		DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
+
+	return drm_connector_attach_encoder(&ms91xx->connector,
+					    &ms91xx->encoder);
+}
diff --git a/drivers/gpu/drm/ms91xx/ms91xx_registers.c b/drivers/gpu/drm/ms91xx/ms91xx_registers.c
new file mode 100644
index 0000000000000..84cbdc78002b1
--- /dev/null
+++ b/drivers/gpu/drm/ms91xx/ms91xx_registers.c
@@ -0,0 +1,386 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <asm/byteorder.h>
+#include <linux/errno.h>
+#include <linux/gfp.h>
+#include <linux/hid.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
+#include <linux/mutex.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_modes.h>
+
+#include "ms91xx.h"
+
+int ms91xx_read_byte(struct ms91xx_device *ms91xx, u16 address)
+{
+	struct ms91xx_request request;
+	struct usb_device *usb_dev;
+	int idx, ret;
+
+	if (!drm_dev_enter(&ms91xx->drm, &idx))
+		return -ENODEV;
+
+	mutex_lock(&ms91xx->ctrl_lock);
+
+	usb_dev = interface_to_usbdev(ms91xx->intf);
+	memset(&request, 0, sizeof(request));
+	request.type = MS91XX_REQ_TYPE_READ_BYTE;
+	request.addr = cpu_to_be16(address);
+
+	ret = usb_control_msg_send(usb_dev, 0, HID_REQ_SET_REPORT,
+				   USB_DIR_OUT | USB_TYPE_CLASS |
+					   USB_RECIP_INTERFACE,
+				   0x0300, 0, &request, sizeof(request),
+				   USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+	if (ret)
+		goto ctrl_unlock;
+
+	ret = usb_control_msg_recv(usb_dev, 0, HID_REQ_GET_REPORT,
+				   USB_DIR_IN | USB_TYPE_CLASS |
+					   USB_RECIP_INTERFACE,
+				   0x0300, 0, &request, sizeof(request),
+				   USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
+	if (!ret)
+		ret = request.data[0];
+
+ctrl_unlock:
+	mutex_unlock(&ms91xx->ctrl_lock);
+	drm_dev_exit(idx);
+	return ret;
+}
+
+static int ms91xx_read_flash(struct ms91xx_device *ms91xx, u32 address,
+			     void *data, size_t len)
+{
+	struct ms91xx_flash_read_request request;
+	struct usb_device *usb_dev;
+	size_t offset = 0;
+	u8 *dst = data;
+	int idx, ret = 0;
+
+	if (address > 0xffffff || len > 0x1000000 - address)
+		return -ERANGE;
+
+	if (!drm_dev_enter(&ms91xx->drm, &idx))
+		return -ENODEV;
+
+	mutex_lock(&ms91xx->ctrl_lock);
+	usb_dev = interface_to_usbdev(ms91xx->intf);
+
+	while (offset < len) {
+		size_t chunk = min_t(size_t, len - offset, sizeof(request));
+		u32 current_address = address + offset;
+
+		memset(&request, 0, sizeof(request));
+		request.type = MS91XX_REQ_TYPE_READ_FLASH;
+		request.addr[0] = current_address >> 16;
+		request.addr[1] = current_address >> 8;
+		request.addr[2] = current_address;
+
+		ret = usb_control_msg_send(usb_dev, 0, HID_REQ_SET_REPORT,
+					   USB_DIR_OUT | USB_TYPE_CLASS |
+						   USB_RECIP_INTERFACE,
+					   0x0300, 0, &request,
+					   sizeof(request),
+					   USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+		if (ret)
+			goto ctrl_unlock;
+
+		ret = usb_control_msg_recv(usb_dev, 0, HID_REQ_GET_REPORT,
+					   USB_DIR_IN | USB_TYPE_CLASS |
+						   USB_RECIP_INTERFACE,
+					   0x0300, 0, &request,
+					   sizeof(request),
+					   USB_CTRL_GET_TIMEOUT, GFP_KERNEL);
+		if (ret)
+			goto ctrl_unlock;
+
+		memcpy(dst + offset, &request, chunk);
+		offset += chunk;
+	}
+
+ctrl_unlock:
+	mutex_unlock(&ms91xx->ctrl_lock);
+	drm_dev_exit(idx);
+	return ret;
+}
+
+static int ms91xx_custom_timing_base(struct ms91xx_device *ms91xx)
+{
+	u8 signature[3];
+	int ret;
+
+	/* Read chip signature */
+	ret = ms91xx_read_byte(ms91xx, MS913X_REG_CHIP_ID);
+	if (ret < 0)
+		return ret;
+	signature[0] = ret;
+
+	ret = ms91xx_read_byte(ms91xx, MS913X_REG_CHIP_ID + 1);
+	if (ret < 0)
+		return ret;
+	signature[1] = ret;
+
+	ret = ms91xx_read_byte(ms91xx, MS913X_REG_CHIP_ID + 2);
+	if (ret < 0)
+		return ret;
+	signature[2] = ret;
+
+	if (signature[1] == MS913X_CHIP_ID_SIGNATURE_MSB &&
+	    signature[2] == MS91XX_CHIP_ID_SIGNATURE_LSB)
+		return MS913X_CUSTOM_TIMING_BASE;
+
+	ret = ms91xx_read_byte(ms91xx, MS912X_REG_CHIP_ID);
+	if (ret < 0)
+		return ret;
+	signature[0] = ret;
+
+	ret = ms91xx_read_byte(ms91xx, MS912X_REG_CHIP_ID + 1);
+	if (ret < 0)
+		return ret;
+	signature[1] = ret;
+
+	ret = ms91xx_read_byte(ms91xx, MS912X_REG_CHIP_ID + 2);
+	if (ret < 0)
+		return ret;
+	signature[2] = ret;
+
+	if (signature[1] == MS912X_CHIP_ID_SIGNATURE_MSB &&
+	    signature[2] == MS91XX_CHIP_ID_SIGNATURE_LSB)
+		return MS912X_CUSTOM_TIMING_BASE;
+
+	return -ENODEV;
+}
+
+static int
+ms91xx_decode_custom_timing(struct ms91xx_custom_mode *custom_mode,
+			    const struct ms91xx_custom_timing_record *record)
+{
+	struct drm_display_mode *display_mode = &custom_mode->display_mode;
+	u16 hsyncwidth = le16_to_cpu(record->hsyncwidth);
+	u16 vsyncwidth = le16_to_cpu(record->vsyncwidth);
+	u16 hoffset = le16_to_cpu(record->hoffset);
+	u16 voffset = le16_to_cpu(record->voffset);
+	u16 hactive = le16_to_cpu(record->hactive);
+	u16 vactive = le16_to_cpu(record->vactive);
+	u16 htotal = le16_to_cpu(record->htotal);
+	u16 vtotal = le16_to_cpu(record->vtotal);
+	u16 pixclk = le16_to_cpu(record->pixclk);
+	u16 vfreq = le16_to_cpu(record->vfreq);
+	unsigned int hblank, vblank;
+	int refresh;
+
+	if (!hactive || hactive > MS91XX_MAX_WIDTH ||
+	    !vactive || vactive > MS91XX_MAX_HEIGHT ||
+	    htotal <= hactive || vtotal <= vactive || !pixclk || !vfreq)
+		return -EINVAL;
+
+	hblank = htotal - hactive;
+	vblank = vtotal - vactive;
+	if (!hsyncwidth || hsyncwidth > hoffset || hoffset > hblank ||
+	    !vsyncwidth || vsyncwidth > voffset || voffset > vblank)
+		return -EINVAL;
+
+	memset(display_mode, 0, sizeof(*display_mode));
+	display_mode->clock = pixclk * 10;
+	display_mode->hdisplay = hactive;
+	display_mode->hsync_start = htotal - hoffset;
+	display_mode->hsync_end = display_mode->hsync_start + hsyncwidth;
+	display_mode->htotal = htotal;
+	display_mode->vdisplay = vactive;
+	display_mode->vsync_start = vtotal - voffset;
+	display_mode->vsync_end = display_mode->vsync_start + vsyncwidth;
+	display_mode->vtotal = vtotal;
+	display_mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	display_mode->flags =
+		record->polarity & MS91XX_TIMING_POSITIVE_HSYNC ?
+			DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
+	display_mode->flags |=
+		record->polarity & MS91XX_TIMING_POSITIVE_VSYNC ?
+			DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
+	if (!(record->polarity & MS91XX_TIMING_PROGRESSIVE))
+		display_mode->flags |= DRM_MODE_FLAG_INTERLACE;
+
+	refresh = DIV_ROUND_CLOSEST(vfreq, 100);
+	if (!refresh || drm_mode_vrefresh(display_mode) != refresh)
+		return -EINVAL;
+
+	drm_mode_set_name(display_mode);
+	custom_mode->mode.width = hactive;
+	custom_mode->mode.height = vactive;
+	custom_mode->mode.hz = refresh;
+	custom_mode->mode.mode = record->vic;
+
+	return 0;
+}
+
+int ms91xx_read_custom_timing(struct ms91xx_device *ms91xx)
+{
+	struct ms91xx_custom_timing_record record;
+	unsigned int num_records, i;
+	int error = 0;
+	u8 marker[7];
+	int base, ret;
+
+	ms91xx->num_custom_modes = 0;
+
+	base = ms91xx_custom_timing_base(ms91xx);
+	if (base < 0)
+		return base;
+
+	ret = ms91xx_read_flash(ms91xx, base, marker, sizeof(marker));
+	if (ret)
+		return ret;
+
+	if (!memcmp(marker, "modify1", sizeof(marker)))
+		num_records = 1;
+	else if (!memcmp(marker, "modify2", sizeof(marker)))
+		num_records = 2;
+	else
+		return 0;
+
+	for (i = 0; i < num_records; i++) {
+		ret = ms91xx_read_flash(ms91xx,
+					base + MS91XX_CUSTOM_TIMING_OFFSET +
+						i * MS91XX_CUSTOM_TIMING_STRIDE,
+					&record, sizeof(record));
+		if (ret) {
+			error = ret;
+			continue;
+		}
+
+		ret = ms91xx_decode_custom_timing(
+			&ms91xx->custom_modes[ms91xx->num_custom_modes],
+			&record);
+		if (ret) {
+			error = ret;
+			continue;
+		}
+
+		ms91xx->num_custom_modes++;
+	}
+
+	return ms91xx->num_custom_modes ? ms91xx->num_custom_modes : error;
+}
+
+static int ms91xx_write_6_bytes(struct ms91xx_device *ms91xx,
+				u8 address, const void *data)
+{
+	struct ms91xx_write_request request;
+	struct usb_device *usb_dev;
+	int idx, ret;
+
+	if (!drm_dev_enter(&ms91xx->drm, &idx))
+		return -ENODEV;
+
+	mutex_lock(&ms91xx->ctrl_lock);
+
+	usb_dev = interface_to_usbdev(ms91xx->intf);
+	request.type = MS91XX_REQ_TYPE_WRITE_6_BYTES;
+	request.addr = address;
+	memcpy(request.data, data, sizeof(request.data));
+
+	ret = usb_control_msg_send(usb_dev, 0, HID_REQ_SET_REPORT,
+				   USB_DIR_OUT | USB_TYPE_CLASS |
+					   USB_RECIP_INTERFACE,
+				   0x0300, 0, &request, sizeof(request),
+				   USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+	mutex_unlock(&ms91xx->ctrl_lock);
+	drm_dev_exit(idx);
+
+	return ret;
+}
+
+int ms91xx_power_on(struct ms91xx_device *ms91xx)
+{
+	u8 data[6];
+
+	memset(data, 0, sizeof(data));
+	data[0] = 0x01;
+	data[1] = 0x02;
+
+	return ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_POWER, data);
+}
+
+int ms91xx_power_off(struct ms91xx_device *ms91xx)
+{
+	u8 data[6];
+
+	memset(data, 0, sizeof(data));
+
+	return ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_POWER, data);
+}
+
+int ms91xx_set_resolution(struct ms91xx_device *ms91xx,
+			  const struct ms91xx_mode *mode)
+{
+	int ret;
+	u8 data[6];
+	struct ms91xx_resolution_request resolution_request;
+	struct ms91xx_mode_request mode_request;
+
+	int width = mode->width;
+	int height = mode->height;
+	int mode_num = mode->mode;
+
+	/* Sequence obtained from Windows USB captures */
+	memset(data, 0, sizeof(data));
+	data[0] = 0;
+	ret = ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_UNKNOWN2, data);
+	if (ret < 0)
+		return ret;
+
+	ret = ms91xx_read_byte(ms91xx, MS91XX_REG_MODE_SEQUENCE_0);
+	if (ret < 0)
+		return ret;
+	ret = ms91xx_read_byte(ms91xx, MS91XX_REG_MODE_SEQUENCE_1);
+	if (ret < 0)
+		return ret;
+	ret = ms91xx_read_byte(ms91xx, MS91XX_REG_MODE_SEQUENCE_2);
+	if (ret < 0)
+		return ret;
+
+	memset(data, 0, sizeof(data));
+	data[0] = 0x03;
+	ret = ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_UNKNOWN1, data);
+	if (ret < 0)
+		return ret;
+
+	/* Write resolution */
+	resolution_request.width = cpu_to_be16(width);
+	resolution_request.height = cpu_to_be16(height);
+	resolution_request.pixel_format = MS91XX_PIXFMT_UYVY;
+	resolution_request.byte_select = MS91XX_BYTE_SELECT_UYVY;
+	ret = ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_RESOLUTION,
+				   &resolution_request);
+	if (ret < 0)
+		return ret;
+
+	/* Write mode */
+	mode_request.mode = mode_num;
+	mode_request.pixel_format = 0x01;
+	mode_request.width = cpu_to_be16(width);
+	mode_request.height = cpu_to_be16(height);
+	ret = ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_MODE, &mode_request);
+	if (ret < 0)
+		return ret;
+
+	memset(data, 0, sizeof(data));
+	data[0] = 1;
+	ret = ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_UNKNOWN2, data);
+	if (ret < 0)
+		return ret;
+
+	memset(data, 0, sizeof(data));
+	data[0] = 1;
+	ret = ms91xx_write_6_bytes(ms91xx, MS91XX_CMD_OUTPUT_ENABLE, data);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
-- 
2.55.0

[PATCH v1 3/4] drm/ms91xx: Add atomic modesetting and USB framebuffer transfers

[Ho Jie Feng]

Implements:
- DRM core code
- Double buffered USB framebuffer transfers
- Pending-transfer replacement with the latest pixels and merged damage
- Cache maintenance for converted vmalloc transfer buffers
- USB driver matching

Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/gpu/drm/ms91xx/ms91xx.h          | 217 ++++++++++
 drivers/gpu/drm/ms91xx/ms91xx_drv.c      | 518 +++++++++++++++++++++++
 drivers/gpu/drm/ms91xx/ms91xx_transfer.c | 218 ++++++++++
 3 files changed, 953 insertions(+)
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx.h
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_drv.c
 create mode 100644 drivers/gpu/drm/ms91xx/ms91xx_transfer.c

diff --git a/drivers/gpu/drm/ms91xx/ms91xx.h b/drivers/gpu/drm/ms91xx/ms91xx.h
new file mode 100644
index 0000000000000..83d3ca96e7b22
--- /dev/null
+++ b/drivers/gpu/drm/ms91xx/ms91xx.h
@@ -0,0 +1,217 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef MS91XX_H
+#define MS91XX_H
+
+#include <linux/bits.h>
+#include <linux/compiler_attributes.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/mutex.h>
+#include <linux/scatterlist.h>
+#include <linux/timer_types.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include <drm/drm_connector.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_device.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_plane.h>
+#include <drm/drm_rect.h>
+
+struct drm_format_conv_state;
+struct drm_framebuffer;
+struct iosys_map;
+
+#define DRIVER_NAME "ms91xx"
+#define DRIVER_DESC "MacroSilicon USB to VGA/HDMI"
+
+#define DRIVER_MAJOR 1
+#define DRIVER_MINOR 0
+
+#define MS91XX_BULK_OUT_ENDPOINT	4
+
+#define MS91XX_REQ_TYPE_WRITE_6_BYTES	0xa6
+#define MS91XX_REQ_TYPE_READ_BYTE	0xb5
+#define MS91XX_REQ_TYPE_READ_FLASH	0xf5
+
+#define MS91XX_REG_VIDEO_PORT		0x0031
+#define MS91XX_REG_DISPLAY_STATUS	0x0032
+#define MS91XX_REG_EDID_BASE		0xc000
+
+#define MS91XX_REG_MODE_SEQUENCE_0	0x0030
+#define MS91XX_REG_MODE_SEQUENCE_1	0x0033
+#define MS91XX_REG_MODE_SEQUENCE_2	0xc620
+
+#define MS91XX_CMD_RESOLUTION		0x01
+#define MS91XX_CMD_MODE			0x02
+#define MS91XX_CMD_UNKNOWN1		0x03
+#define MS91XX_CMD_UNKNOWN2		0x04
+#define MS91XX_CMD_OUTPUT_ENABLE	0x05
+#define MS91XX_CMD_POWER		0x07
+
+#define MS913X_REG_CHIP_ID			0xff00
+#define MS912X_REG_CHIP_ID			0xf000
+#define MS913X_CHIP_ID_SIGNATURE_MSB		0x13
+#define MS912X_CHIP_ID_SIGNATURE_MSB		0x16
+#define MS91XX_CHIP_ID_SIGNATURE_LSB		0x0a
+#define MS913X_CUSTOM_TIMING_BASE		0xfc50
+#define MS912X_CUSTOM_TIMING_BASE		0x1c00
+#define MS91XX_CUSTOM_TIMING_OFFSET		0x10
+#define MS91XX_CUSTOM_TIMING_STRIDE		0x20
+
+#define MS91XX_TIMING_PROGRESSIVE		BIT(0)
+#define MS91XX_TIMING_POSITIVE_HSYNC		BIT(1)
+#define MS91XX_TIMING_POSITIVE_VSYNC		BIT(2)
+
+enum ms91xx_video_port {
+	MS91XX_VIDEO_PORT_CVBS = 0,
+	MS91XX_VIDEO_PORT_SVIDEO = 1,
+	MS91XX_VIDEO_PORT_VGA = 2,
+	MS91XX_VIDEO_PORT_YPBPR = 3,
+	MS91XX_VIDEO_PORT_CVBS_SVIDEO = 4,
+	MS91XX_VIDEO_PORT_HDMI = 5,
+	MS91XX_VIDEO_PORT_DIGITAL = 6,
+	MS91XX_VIDEO_PORT_UNKNOWN = 0xff,
+};
+
+struct ms91xx_usb_request {
+	void *transfer_buffer;
+	struct ms91xx_device *ms91xx;
+	size_t transfer_len;
+	struct drm_rect transfer_rect;
+	struct sg_table transfer_sgt;
+	struct usb_sg_request sgr;
+	struct work_struct work;
+	struct timer_list timer;
+	struct completion done;
+};
+
+struct ms91xx_mode {
+	int width;
+	int height;
+	int hz;
+	int mode;
+};
+
+struct ms91xx_custom_mode {
+	struct ms91xx_mode mode;
+	struct drm_display_mode display_mode;
+};
+
+#define MS91XX_PIXFMT_RGB888		0x11
+#define MS91XX_PIXFMT_UYVY		0x22
+#define MS91XX_BYTE_SELECT_UYVY		0x00
+
+#define MS91XX_MODE(w, h, z, m)                                               \
+	{                                                                      \
+		.width = w, .height = h, .hz = z, .mode = m                    \
+	}
+
+struct ms91xx_device {
+	struct drm_device drm;
+	struct usb_interface *intf;
+	unsigned int bulk_pipe;
+	enum ms91xx_video_port port_type;
+	struct workqueue_struct *workqueue;
+	/* Serializes HID control request/response sequences */
+	struct mutex ctrl_lock;
+
+	struct drm_connector connector;
+	struct drm_encoder encoder;
+	struct drm_crtc crtc;
+	struct drm_plane plane;
+
+	struct ms91xx_custom_mode custom_modes[2];
+	unsigned int num_custom_modes;
+
+	struct drm_rect update_rect;
+
+	/* Double buffer to allow memcpy and transfer
+	 * to happen in parallel
+	 */
+	int current_request;
+	struct ms91xx_usb_request requests[2];
+};
+
+struct ms91xx_request {
+	u8 type;
+	__be16 addr;
+	u8 data[5];
+} __packed;
+
+struct ms91xx_write_request {
+	u8 type;
+	u8 addr;
+	u8 data[6];
+} __packed;
+
+struct ms91xx_resolution_request {
+	__be16 width;
+	__be16 height;
+	u8 pixel_format;
+	u8 byte_select;
+} __packed;
+
+struct ms91xx_mode_request {
+	u8 mode;
+	u8 pixel_format;
+	__be16 width;
+	__be16 height;
+} __packed;
+
+struct ms91xx_frame_update_header {
+	__be16 marker;
+	u8 position[3]; /* x:y, packed 12-bit big-endian values */
+	u8 dimensions[3]; /* width:height, packed 12-bit big-endian values */
+} __packed;
+
+struct ms91xx_flash_read_request {
+	u8 type;
+	u8 addr[3];
+	u8 reserved[4];
+} __packed;
+
+struct ms91xx_custom_timing_record {
+	u8 vic;
+	u8 polarity;
+	__le16 htotal;
+	__le16 vtotal;
+	__le16 hactive;
+	__le16 vactive;
+	__le16 pixclk;
+	__le16 vfreq;
+	__le16 hoffset;
+	__le16 voffset;
+	__le16 hsyncwidth;
+	__le16 vsyncwidth;
+} __packed;
+
+#define MS91XX_FRAME_OVERHEAD 16
+#define MS91XX_MAX_WIDTH 1920
+#define MS91XX_MAX_HEIGHT 1200
+#define MS91XX_MAX_TRANSFER_LEN \
+	(MS91XX_MAX_WIDTH * MS91XX_MAX_HEIGHT * 2 + MS91XX_FRAME_OVERHEAD)
+
+#define to_ms91xx(x) container_of(x, struct ms91xx_device, drm)
+
+int ms91xx_read_byte(struct ms91xx_device *ms91xx, u16 address);
+int ms91xx_read_custom_timing(struct ms91xx_device *ms91xx);
+int ms91xx_connector_init(struct ms91xx_device *ms91xx);
+int ms91xx_set_resolution(struct ms91xx_device *ms91xx,
+			  const struct ms91xx_mode *mode);
+
+int ms91xx_power_on(struct ms91xx_device *ms91xx);
+int ms91xx_power_off(struct ms91xx_device *ms91xx);
+
+int ms91xx_fb_send_rect(struct drm_framebuffer *fb, const struct iosys_map *map,
+			struct drm_format_conv_state *fmtcnv_state,
+			struct drm_rect *rect);
+
+void ms91xx_free_request(struct ms91xx_usb_request *request);
+int ms91xx_init_request(struct ms91xx_device *ms91xx,
+			struct ms91xx_usb_request *request, size_t len);
+#endif
diff --git a/drivers/gpu/drm/ms91xx/ms91xx_drv.c b/drivers/gpu/drm/ms91xx/ms91xx_drv.c
new file mode 100644
index 0000000000000..2ca855f2363c8
--- /dev/null
+++ b/drivers/gpu/drm/ms91xx/ms91xx_drv.c
@@ -0,0 +1,518 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/array_size.h>
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/limits.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/types.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include <drm/clients/drm_client_setup.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_damage_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_fbdev_shmem.h>
+#include <drm/drm_file.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_ioctl.h>
+#include <drm/drm_managed.h>
+#include <drm/drm_mode_config.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_modeset_helper.h>
+#include <drm/drm_modeset_helper_vtables.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_print.h>
+#include <drm/drm_rect.h>
+
+#include "ms91xx.h"
+
+static int ms91xx_usb_suspend(struct usb_interface *interface,
+			      pm_message_t message)
+{
+	struct ms91xx_device *ms91xx = usb_get_intfdata(interface);
+
+	return drm_mode_config_helper_suspend(&ms91xx->drm);
+}
+
+static int ms91xx_usb_resume(struct usb_interface *interface)
+{
+	struct ms91xx_device *ms91xx = usb_get_intfdata(interface);
+
+	return drm_mode_config_helper_resume(&ms91xx->drm);
+}
+
+DEFINE_DRM_GEM_FOPS(ms91xx_driver_fops);
+
+static const struct drm_driver ms91xx_drm_driver = {
+	.driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET,
+
+	/* GEM hooks */
+	.fops = &ms91xx_driver_fops,
+	DRM_GEM_SHMEM_DRIVER_OPS,
+	DRM_FBDEV_SHMEM_DRIVER_OPS,
+
+	.name = DRIVER_NAME,
+	.desc = DRIVER_DESC,
+	.major = DRIVER_MAJOR,
+	.minor = DRIVER_MINOR,
+};
+
+static const struct drm_mode_config_funcs ms91xx_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static const struct drm_mode_config_helper_funcs
+ms91xx_mode_config_helper_funcs = {
+	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+};
+
+static const struct ms91xx_mode ms91xx_mode_list[] = {
+	/* Found in captures of the Windows driver and dumped from device */
+	MS91XX_MODE(720,   480, 60, 0x02), /* 60.00 Hz */
+	MS91XX_MODE(720,   576, 50, 0x11), /* 50.00 Hz */
+	MS91XX_MODE(1280,  720, 50, 0x13), /* 50.00 Hz */
+	MS91XX_MODE(1920, 1080, 50, 0x1f), /* 50.00 Hz */
+	MS91XX_MODE(1920, 1080, 30, 0x22), /* 30.00 Hz */
+	MS91XX_MODE(640,   480, 60, 0x40), /* 59.94 Hz */
+	MS91XX_MODE(800,   600, 60, 0x42), /* 60.32 Hz */
+	MS91XX_MODE(800,   600, 75, 0x44), /* 75.00 Hz */
+	MS91XX_MODE(1024,  768, 60, 0x47), /* 60.00 Hz */
+	MS91XX_MODE(1024,  768, 75, 0x49), /* 75.03 Hz */
+	MS91XX_MODE(1152,  864, 60, 0x4c), /* 60.00 Hz */
+	MS91XX_MODE(1280,  600, 60, 0x4e), /* 60.00 Hz */
+	MS91XX_MODE(1280,  720, 60, 0x4f), /* 60.00 Hz */
+	MS91XX_MODE(1280,  768, 60, 0x54), /* 59.87 Hz */
+	MS91XX_MODE(1280,  768, 75, 0x56), /* 74.89 Hz */
+	MS91XX_MODE(1280,  800, 60, 0x57), /* 59.81 Hz */
+	MS91XX_MODE(1280,  960, 60, 0x5b), /* 60.00 Hz */
+	MS91XX_MODE(1280, 1024, 60, 0x60), /* 60.02 Hz */
+	MS91XX_MODE(1280, 1024, 75, 0x61), /* 75.02 Hz */
+	MS91XX_MODE(1360,  768, 60, 0x64), /* 60.02 Hz */
+	MS91XX_MODE(1366,  768, 60, 0x66), /* 59.79 Hz */
+	MS91XX_MODE(1400, 1050, 60, 0x67), /* 59.98 Hz */
+	MS91XX_MODE(1440,  900, 60, 0x6b), /* 59.89 Hz */
+	MS91XX_MODE(1600, 1200, 60, 0x73), /* 60.00 Hz */
+	MS91XX_MODE(1680, 1050, 60, 0x78), /* 59.95 Hz */
+	MS91XX_MODE(1920, 1080, 60, 0x81), /* 60.00 Hz */
+};
+
+static const struct ms91xx_mode *
+ms91xx_get_mode(struct ms91xx_device *ms91xx,
+		const struct drm_display_mode *mode)
+{
+	unsigned int i;
+	int width = mode->hdisplay;
+	int height = mode->vdisplay;
+	int hz = drm_mode_vrefresh(mode);
+
+	for (i = 0; i < ms91xx->num_custom_modes; i++) {
+		const struct ms91xx_mode *custom_mode =
+			&ms91xx->custom_modes[i].mode;
+
+		if (custom_mode->width == width &&
+		    custom_mode->height == height &&
+		    custom_mode->hz == hz)
+			return custom_mode;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(ms91xx_mode_list); i++) {
+		if (ms91xx_mode_list[i].width == width &&
+		    ms91xx_mode_list[i].height == height &&
+		    ms91xx_mode_list[i].hz == hz) {
+			return &ms91xx_mode_list[i];
+		}
+	}
+	return NULL;
+}
+
+static void ms91xx_crtc_atomic_enable(struct drm_crtc *crtc,
+				      struct drm_atomic_commit *state)
+{
+	struct drm_crtc_state *crtc_state =
+		drm_atomic_get_new_crtc_state(state, crtc);
+	struct drm_device *dev = crtc->dev;
+	struct ms91xx_device *ms91xx = to_ms91xx(dev);
+	const struct ms91xx_mode *ms_mode;
+	int ret;
+
+	ret = ms91xx_power_on(ms91xx);
+	if (ret) {
+		drm_err(dev, "failed to power on display: %d\n", ret);
+		return;
+	}
+
+	ms_mode = ms91xx_get_mode(ms91xx, &crtc_state->mode);
+	if (!ms_mode) {
+		drm_err(dev, "unsupported mode passed to CRTC enable\n");
+		return;
+	}
+
+	ret = ms91xx_set_resolution(ms91xx, ms_mode);
+	if (ret)
+		drm_err(dev, "failed to set display mode: %d\n", ret);
+}
+
+static void ms91xx_cancel_transfer_work(struct ms91xx_device *ms91xx)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(ms91xx->requests); i++) {
+		struct ms91xx_usb_request *request = &ms91xx->requests[i];
+
+		if (cancel_work_sync(&request->work))
+			complete(&request->done);
+	}
+}
+
+static void ms91xx_crtc_atomic_disable(struct drm_crtc *crtc,
+				       struct drm_atomic_commit *state)
+{
+	struct drm_device *dev = crtc->dev;
+	struct ms91xx_device *ms91xx = to_ms91xx(dev);
+	int ret;
+
+	ms91xx_cancel_transfer_work(ms91xx);
+	ret = ms91xx_power_off(ms91xx);
+	if (ret && ret != -ENODEV)
+		drm_err(dev, "failed to power off display: %d\n", ret);
+}
+
+static enum drm_mode_status
+ms91xx_crtc_mode_valid(struct drm_crtc *crtc,
+		       const struct drm_display_mode *mode)
+{
+	struct ms91xx_device *ms91xx = to_ms91xx(crtc->dev);
+	const struct ms91xx_mode *ms_mode = ms91xx_get_mode(ms91xx, mode);
+
+	if (!ms_mode)
+		return MODE_BAD;
+
+	return MODE_OK;
+}
+
+static int ms91xx_plane_atomic_check(struct drm_plane *plane,
+				     struct drm_atomic_commit *state)
+{
+	struct drm_plane_state *old_plane_state;
+	struct drm_plane_state *new_plane_state;
+	struct drm_crtc_state *crtc_state = NULL;
+
+	old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	if (new_plane_state->crtc)
+		crtc_state = drm_atomic_get_new_crtc_state(state,
+							   new_plane_state->crtc);
+
+	/*
+	 * A mode set does change how the device interprets the framebuffer
+	 * so ask for a full plane update for the first frame after one.
+	 */
+	new_plane_state->ignore_damage_clips =
+		old_plane_state->fb != new_plane_state->fb ||
+		(crtc_state && drm_atomic_crtc_needs_modeset(crtc_state));
+
+	return drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
+						   DRM_PLANE_NO_SCALING,
+						   DRM_PLANE_NO_SCALING,
+						   false, false);
+}
+
+static void ms91xx_clear_rect(struct drm_rect *rect)
+{
+	rect->x1 = INT_MAX;
+	rect->y1 = INT_MAX;
+	rect->x2 = 0;
+	rect->y2 = 0;
+}
+
+static void ms91xx_merge_rects(struct drm_rect *dest, struct drm_rect *r1,
+			       struct drm_rect *r2)
+{
+	dest->x1 = min(r1->x1, r2->x1);
+	dest->y1 = min(r1->y1, r2->y1);
+	dest->x2 = max(r1->x2, r2->x2);
+	dest->y2 = max(r1->y2, r2->y2);
+}
+
+static void ms91xx_plane_atomic_update(struct drm_plane *plane,
+				       struct drm_atomic_commit *state)
+{
+	struct drm_plane_state *old_plane_state;
+	struct drm_plane_state *new_plane_state;
+	struct drm_crtc_state *new_crtc_state;
+	struct drm_shadow_plane_state *shadow_plane_state;
+	struct ms91xx_device *ms91xx = to_ms91xx(plane->dev);
+	struct ms91xx_usb_request *current_request, *previous_request;
+	struct drm_rect current_rect, rect;
+
+	old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+	new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+
+	if (!new_plane_state->fb)
+		return;
+	new_crtc_state = drm_atomic_get_new_crtc_state(state,
+						       new_plane_state->crtc);
+	shadow_plane_state = to_drm_shadow_plane_state(new_plane_state);
+
+	/* Damage from the previous mode may not fit the new framebuffer. */
+	if (drm_atomic_crtc_needs_modeset(new_crtc_state))
+		ms91xx_clear_rect(&ms91xx->update_rect);
+
+	if (drm_atomic_helper_damage_merged(old_plane_state, new_plane_state,
+					    &current_rect)) {
+		current_request = &ms91xx->requests[ms91xx->current_request];
+		previous_request = &ms91xx->requests[!ms91xx->current_request];
+
+		/* Latency reduction: requeue with the latest frame data. */
+		if (cancel_work(&previous_request->work)) {
+			complete(&previous_request->done);
+			ms91xx_merge_rects(&current_rect, &current_rect,
+					   &previous_request->transfer_rect);
+			current_request = previous_request;
+			ms91xx->current_request = !ms91xx->current_request;
+		}
+
+		current_request->transfer_rect = current_rect;
+
+		/*
+		 * The device double buffers, so we need to send the update
+		 * rects of the last two frames.
+		 */
+		ms91xx_merge_rects(&rect, &current_rect, &ms91xx->update_rect);
+		if (ms91xx_fb_send_rect(new_plane_state->fb,
+					&shadow_plane_state->data[0],
+					&shadow_plane_state->fmtcnv_state,
+					&rect)) {
+			/* In case of error, merge the rects to update later */
+			ms91xx_merge_rects(&ms91xx->update_rect,
+					   &ms91xx->update_rect, &rect);
+		} else {
+			ms91xx->update_rect = current_rect;
+		}
+	}
+}
+
+static const struct drm_crtc_helper_funcs ms91xx_crtc_helper_funcs = {
+	.mode_valid = ms91xx_crtc_mode_valid,
+	.atomic_check = drm_crtc_helper_atomic_check,
+	.atomic_enable = ms91xx_crtc_atomic_enable,
+	.atomic_disable = ms91xx_crtc_atomic_disable,
+};
+
+static const struct drm_crtc_funcs ms91xx_crtc_funcs = {
+	.atomic_create_state = drm_atomic_helper_crtc_create_state,
+	.destroy = drm_crtc_cleanup,
+	.set_config = drm_atomic_helper_set_config,
+	.page_flip = drm_atomic_helper_page_flip,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+};
+
+static const struct drm_plane_helper_funcs ms91xx_plane_helper_funcs = {
+	DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+	.atomic_check = ms91xx_plane_atomic_check,
+	.atomic_update = ms91xx_plane_atomic_update,
+};
+
+static const struct drm_plane_funcs ms91xx_plane_funcs = {
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
+	.destroy = drm_plane_cleanup,
+	DRM_GEM_SHADOW_PLANE_FUNCS,
+};
+
+static const u32 ms91xx_plane_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
+
+static const u8 ms91xx_bulk_out_endpoints[] = {
+	MS91XX_BULK_OUT_ENDPOINT, 0
+};
+
+static int ms91xx_usb_probe(struct usb_interface *interface,
+			    const struct usb_device_id *id)
+{
+	int ret;
+	unsigned int i;
+	struct ms91xx_device *ms91xx;
+	struct drm_device *dev;
+	struct device *dma_dev;
+	struct usb_device *usbdev;
+
+	ms91xx = devm_drm_dev_alloc(&interface->dev, &ms91xx_drm_driver,
+				    struct ms91xx_device, drm);
+	if (IS_ERR(ms91xx))
+		return PTR_ERR(ms91xx);
+
+	ms91xx->intf = interface;
+	ret = devm_mutex_init(&interface->dev, &ms91xx->ctrl_lock);
+	if (ret)
+		return ret;
+
+	if (!usb_check_bulk_endpoints(interface, ms91xx_bulk_out_endpoints))
+		return -ENXIO;
+
+	ret = ms91xx_read_custom_timing(ms91xx);
+	if (ret < 0) {
+		drm_warn(&ms91xx->drm,
+			 "failed to read custom timing: %d\n", ret);
+	}
+	for (i = 0; i < ms91xx->num_custom_modes; i++) {
+		const struct ms91xx_mode *mode = &ms91xx->custom_modes[i].mode;
+
+		drm_info(&ms91xx->drm,
+			 "custom mode %dx%d@%d uses mode 0x%02x\n",
+			 mode->width, mode->height, mode->hz, mode->mode);
+	}
+
+	usbdev = interface_to_usbdev(interface);
+	ms91xx->bulk_pipe =
+		usb_sndbulkpipe(usbdev, MS91XX_BULK_OUT_ENDPOINT);
+	dev = &ms91xx->drm;
+
+	dma_dev = usb_intf_get_dma_device(interface);
+	if (dma_dev) {
+		drm_dev_set_dma_dev(dev, dma_dev);
+		put_device(dma_dev);
+	} else {
+		drm_warn(dev,
+			 "buffer sharing not supported\n"); /* not an error */
+	}
+	ret = drmm_mode_config_init(dev);
+	if (ret)
+		return ret;
+
+	dev->mode_config.min_width = 0;
+	dev->mode_config.max_width = MS91XX_MAX_WIDTH;
+	dev->mode_config.min_height = 0;
+	dev->mode_config.max_height = MS91XX_MAX_HEIGHT;
+	dev->mode_config.funcs = &ms91xx_mode_config_funcs;
+	dev->mode_config.helper_private = &ms91xx_mode_config_helper_funcs;
+
+	/* This stops weird behavior in the device */
+	ret = ms91xx_set_resolution(ms91xx, &ms91xx_mode_list[0]);
+	if (ret)
+		return ret;
+
+	ms91xx->workqueue =
+		drmm_alloc_ordered_workqueue(dev, DRIVER_NAME, 0);
+	if (IS_ERR(ms91xx->workqueue))
+		return PTR_ERR(ms91xx->workqueue);
+
+	ret = ms91xx_init_request(ms91xx, &ms91xx->requests[0],
+				  MS91XX_MAX_TRANSFER_LEN);
+	if (ret)
+		return ret;
+
+	ret = ms91xx_init_request(ms91xx, &ms91xx->requests[1],
+				  MS91XX_MAX_TRANSFER_LEN);
+	if (ret)
+		goto err_free_request_0;
+
+	ret = drm_universal_plane_init(dev, &ms91xx->plane, 0,
+				       &ms91xx_plane_funcs,
+				       ms91xx_plane_formats,
+				       ARRAY_SIZE(ms91xx_plane_formats), NULL,
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret)
+		goto err_free_request_1;
+
+	drm_plane_helper_add(&ms91xx->plane, &ms91xx_plane_helper_funcs);
+	drm_plane_enable_fb_damage_clips(&ms91xx->plane);
+	ms91xx_clear_rect(&ms91xx->update_rect);
+
+	ret = drm_crtc_init_with_planes(dev, &ms91xx->crtc, &ms91xx->plane,
+					NULL, &ms91xx_crtc_funcs, NULL);
+	if (ret)
+		goto err_free_request_1;
+
+	drm_crtc_helper_add(&ms91xx->crtc, &ms91xx_crtc_helper_funcs);
+
+	ret = ms91xx_connector_init(ms91xx);
+	if (ret)
+		goto err_free_request_1;
+
+	drm_mode_config_reset(dev);
+
+	usb_set_intfdata(interface, ms91xx);
+
+	drmm_kms_helper_poll_init(dev);
+
+	ret = drm_dev_register(dev, 0);
+	if (ret)
+		goto err_free_request_1;
+
+	drm_client_setup(dev, NULL);
+
+	return 0;
+
+err_free_request_1:
+	ms91xx_free_request(&ms91xx->requests[1]);
+err_free_request_0:
+	ms91xx_free_request(&ms91xx->requests[0]);
+	return ret;
+}
+
+static void ms91xx_usb_disconnect(struct usb_interface *interface)
+{
+	struct ms91xx_device *ms91xx = usb_get_intfdata(interface);
+	struct drm_device *dev = &ms91xx->drm;
+
+	drm_dev_unplug(dev);
+	drm_atomic_helper_shutdown(dev);
+	ms91xx_cancel_transfer_work(ms91xx);
+	ms91xx_free_request(&ms91xx->requests[0]);
+	ms91xx_free_request(&ms91xx->requests[1]);
+}
+
+static void ms91xx_usb_shutdown(struct usb_interface *interface)
+{
+	struct ms91xx_device *ms91xx = usb_get_intfdata(interface);
+
+	drm_atomic_helper_shutdown(&ms91xx->drm);
+}
+
+static const struct usb_device_id id_table[] = {
+	/* USB 2 */
+	{ USB_DEVICE_AND_INTERFACE_INFO(0x534d, 0x6021, 0xff, 0x00, 0x00) },
+	/* USB 2 Sometimes this PID will pop up */
+	{ USB_DEVICE_AND_INTERFACE_INFO(0x534d, 0x0821, 0xff, 0x00, 0x00) },
+	/* USB 3 */
+	{ USB_DEVICE_AND_INTERFACE_INFO(0x345f, 0x9132, 0xff, 0x00, 0x00) },
+	{ USB_DEVICE_AND_INTERFACE_INFO(0x345f, 0x9133, 0xff, 0x00, 0x00) },
+	{ USB_DEVICE_AND_INTERFACE_INFO(0x345f, 0x9135, 0xff, 0x00, 0x00) },
+	{ }
+};
+MODULE_DEVICE_TABLE(usb, id_table);
+
+static struct usb_driver ms91xx_driver = {
+	.name = DRIVER_NAME,
+	.probe = ms91xx_usb_probe,
+	.disconnect = ms91xx_usb_disconnect,
+	.suspend = ms91xx_usb_suspend,
+	.resume = ms91xx_usb_resume,
+	.reset_resume = ms91xx_usb_resume,
+	.shutdown = ms91xx_usb_shutdown,
+	.id_table = id_table,
+};
+module_usb_driver(ms91xx_driver);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/ms91xx/ms91xx_transfer.c b/drivers/gpu/drm/ms91xx/ms91xx_transfer.c
new file mode 100644
index 0000000000000..8b88b2206ce3d
--- /dev/null
+++ b/drivers/gpu/drm/ms91xx/ms91xx_transfer.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <asm/byteorder.h>
+#include <linux/align.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/dma-direction.h>
+#include <linux/errno.h>
+#include <linux/gfp.h>
+#include <linux/highmem.h>
+#include <linux/iosys-map.h>
+#include <linux/jiffies.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
+#include <linux/mm.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/timer.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/usb.h>
+#include <linux/vmalloc.h>
+#include <linux/workqueue.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_format_helper.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_print.h>
+#include <drm/drm_rect.h>
+
+#include "ms91xx.h"
+
+static void ms91xx_request_timeout(struct timer_list *t)
+{
+	struct ms91xx_usb_request *request = timer_container_of(request, t, timer);
+
+	usb_sg_cancel(&request->sgr);
+}
+
+static void ms91xx_request_work(struct work_struct *work)
+{
+	struct ms91xx_usb_request *request =
+		container_of(work, struct ms91xx_usb_request, work);
+	struct ms91xx_device *ms91xx = request->ms91xx;
+	struct usb_device *usbdev = interface_to_usbdev(ms91xx->intf);
+	struct usb_sg_request *sgr = &request->sgr;
+	struct sg_table *transfer_sgt = &request->transfer_sgt;
+	int idx, ret;
+
+	if (!drm_dev_enter(&ms91xx->drm, &idx)) {
+		ret = -ENODEV;
+		goto complete;
+	}
+
+	ret = usb_sg_init(sgr, usbdev, ms91xx->bulk_pipe, 0, transfer_sgt->sgl,
+			  transfer_sgt->nents, request->transfer_len,
+			  GFP_KERNEL);
+	if (ret)
+		goto dev_exit;
+
+	mod_timer(&request->timer, jiffies + msecs_to_jiffies(5000));
+	usb_sg_wait(sgr);
+
+	if (!timer_delete_sync(&request->timer))
+		ret = -ETIMEDOUT;
+	else if (sgr->status < 0)
+		ret = sgr->status;
+	else if (sgr->bytes != request->transfer_len)
+		ret = -EIO;
+
+dev_exit:
+	drm_dev_exit(idx);
+complete:
+	if (ret < 0 && ret != -ENODEV)
+		drm_err_ratelimited(&ms91xx->drm,
+				    "failed to send framebuffer: %d\n", ret);
+	complete(&request->done);
+}
+
+void ms91xx_free_request(struct ms91xx_usb_request *request)
+{
+	if (!request->transfer_buffer)
+		return;
+
+	timer_shutdown_sync(&request->timer);
+	sg_free_table(&request->transfer_sgt);
+	vfree(request->transfer_buffer);
+	request->transfer_buffer = NULL;
+}
+
+int ms91xx_init_request(struct ms91xx_device *ms91xx,
+			struct ms91xx_usb_request *request, size_t len)
+{
+	int ret;
+	unsigned int i, num_pages;
+	void *data;
+	struct page **pages;
+	void *ptr;
+
+	data = vmalloc_32(len);
+	if (!data)
+		return -ENOMEM;
+
+	num_pages = DIV_ROUND_UP(len, PAGE_SIZE);
+	pages = kmalloc_array(num_pages, sizeof(struct page *), GFP_KERNEL);
+	if (!pages) {
+		ret = -ENOMEM;
+		goto err_vfree;
+	}
+
+	for (i = 0, ptr = data; i < num_pages; i++, ptr += PAGE_SIZE)
+		pages[i] = vmalloc_to_page(ptr);
+	ret = sg_alloc_table_from_pages(&request->transfer_sgt, pages,
+					num_pages, 0, len, GFP_KERNEL);
+	kfree(pages);
+	if (ret)
+		goto err_vfree;
+
+	request->transfer_buffer = data;
+	request->ms91xx = ms91xx;
+
+	init_completion(&request->done);
+	complete(&request->done);
+	timer_setup(&request->timer, ms91xx_request_timeout, 0);
+	INIT_WORK(&request->work, ms91xx_request_work);
+	return 0;
+
+err_vfree:
+	vfree(data);
+	return ret;
+}
+
+static const u8 ms91xx_end_of_buffer[] = { 0xff, 0xc0, 0x00, 0x00,
+				    0x00, 0x00, 0x00, 0x00 };
+
+static void ms91xx_fb_xrgb8888_to_yuv422(void *dst,
+					 const struct iosys_map *src,
+					 struct drm_framebuffer *fb,
+					 const struct drm_rect *rect,
+					 struct drm_format_conv_state *fmtcnv_state)
+{
+	struct ms91xx_frame_update_header *header = dst;
+	struct iosys_map dst_map;
+	u32 position, dimensions;
+	int x, y1, width;
+
+	y1 = rect->y1;
+	x = rect->x1;
+	width = drm_rect_width(rect);
+
+	header->marker = cpu_to_be16(0xff00);
+	position = ((x & 0xfff) << 12) | (y1 & 0xfff);
+	dimensions = ((width & 0xfff) << 12) |
+		     (drm_rect_height(rect) & 0xfff);
+	put_unaligned_be24(position, header->position);
+	put_unaligned_be24(dimensions, header->dimensions);
+	dst += sizeof(*header);
+
+	iosys_map_set_vaddr(&dst_map, dst);
+	drm_fb_xrgb8888_to_uyvy(&dst_map, NULL, src, fb, rect, fmtcnv_state);
+	dst += width * drm_rect_height(rect) * 2;
+
+	memcpy(dst, ms91xx_end_of_buffer, sizeof(ms91xx_end_of_buffer));
+}
+
+int ms91xx_fb_send_rect(struct drm_framebuffer *fb, const struct iosys_map *map,
+			struct drm_format_conv_state *fmtcnv_state,
+			struct drm_rect *rect)
+{
+	int ret = 0, idx;
+	struct ms91xx_device *ms91xx = to_ms91xx(fb->dev);
+	struct drm_device *drm = &ms91xx->drm;
+	struct ms91xx_usb_request *current_request;
+	int x, width;
+
+	/* UYVY stores pixels in pairs. Expand damage to a complete pair. */
+	x = ALIGN_DOWN(rect->x1, 2);
+	width = min_t(int, ALIGN(rect->x2, 2), fb->width) - x;
+	rect->x1 = x;
+	rect->x2 = x + width;
+	rect->y2 = min_t(unsigned int, rect->y2, fb->height);
+	current_request = &ms91xx->requests[ms91xx->current_request];
+
+	if (!drm_dev_enter(drm, &idx))
+		return -ENODEV;
+
+	/* Transfer buffer still in use, drop this frame. */
+	if (!wait_for_completion_timeout(&current_request->done,
+					 msecs_to_jiffies(10))) {
+		ret = -ETIMEDOUT;
+		goto dev_exit;
+	}
+
+	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
+	if (ret < 0)
+		goto request_complete;
+
+	ms91xx_fb_xrgb8888_to_yuv422(current_request->transfer_buffer,
+				     map, fb, rect, fmtcnv_state);
+	current_request->transfer_len =
+		width * 2 * drm_rect_height(rect) + MS91XX_FRAME_OVERHEAD;
+	flush_kernel_vmap_range(current_request->transfer_buffer,
+				current_request->transfer_len);
+
+	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+
+	queue_work(ms91xx->workqueue, &current_request->work);
+	ms91xx->current_request = 1 - ms91xx->current_request;
+	goto dev_exit;
+
+request_complete:
+	complete(&current_request->done);
+dev_exit:
+	drm_dev_exit(idx);
+	return ret;
+}
-- 
2.55.0

[PATCH v1 4/4] drm/ms91xx: Add Kconfig and Makefile integration

[Ho Jie Feng]

Enable building of the ms91xx driver

Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 MAINTAINERS                     |  7 +++++++
 drivers/gpu/drm/Kconfig         |  1 +
 drivers/gpu/drm/Makefile        |  1 +
 drivers/gpu/drm/ms91xx/Kconfig  | 13 +++++++++++++
 drivers/gpu/drm/ms91xx/Makefile |  9 +++++++++
 5 files changed, 31 insertions(+)
 create mode 100644 drivers/gpu/drm/ms91xx/Kconfig
 create mode 100644 drivers/gpu/drm/ms91xx/Makefile

diff --git a/MAINTAINERS b/MAINTAINERS
index a08087fb6e678..b7be4261bc054 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8243,6 +8243,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/display/panel/lxd,m9189a.yaml
 F:	drivers/gpu/drm/panel/panel-lxd-m9189a.c
 
+DRM DRIVER FOR MACROSILICON MS912X/MS913X USB DISPLAY ADAPTERS
+M:	Ho Jie Feng <hjf3108@gmail.com>
+L:	dri-devel@lists.freedesktop.org
+S:	Maintained
+T:	git https://gitlab.freedesktop.org/drm/misc/kernel.git
+F:	drivers/gpu/drm/ms91xx/
+
 DRM DRIVER FOR MANTIX MLAF057WE51 PANELS
 M:	Guido Günther <agx@sigxcpu.org>
 R:	Purism Kernel Team <kernel@puri.sm>
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 61111d50b1c65..93d73c7b90ce1 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -330,6 +330,7 @@ source "drivers/gpu/drm/mcde/Kconfig"
 source "drivers/gpu/drm/mediatek/Kconfig"
 source "drivers/gpu/drm/meson/Kconfig"
 source "drivers/gpu/drm/mgag200/Kconfig"
+source "drivers/gpu/drm/ms91xx/Kconfig"
 source "drivers/gpu/drm/msm/Kconfig"
 source "drivers/gpu/drm/mxsfb/Kconfig"
 source "drivers/gpu/drm/nouveau/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 2c6bc093a923a..8e98b7ac89f2a 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -191,6 +191,7 @@ obj-$(CONFIG_DRM_EXYNOS) +=exynos/
 obj-$(CONFIG_DRM_ROCKCHIP) +=rockchip/
 obj-$(CONFIG_DRM_GMA500) += gma500/
 obj-$(CONFIG_DRM_UDL) += udl/
+obj-$(CONFIG_DRM_MS91XX) += ms91xx/
 obj-$(CONFIG_DRM_AST) += ast/
 obj-$(CONFIG_DRM_ARMADA) += armada/
 obj-$(CONFIG_DRM_ATMEL_HLCDC)	+= atmel-hlcdc/
diff --git a/drivers/gpu/drm/ms91xx/Kconfig b/drivers/gpu/drm/ms91xx/Kconfig
new file mode 100644
index 0000000000000..526aaa2ee5504
--- /dev/null
+++ b/drivers/gpu/drm/ms91xx/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config DRM_MS91XX
+	tristate "MacroSilicon MS912x/MS913x USB display adapters"
+	depends on DRM && USB && MMU
+	select DRM_CLIENT_SELECTION
+	select DRM_GEM_SHMEM_HELPER
+	select DRM_KMS_HELPER
+	help
+	  This is a KMS driver for MacroSilicon MS912x/MS913x USB display
+	  adapters with VGA or HDMI outputs.
+
+	  If M is selected the module will be called ms91xx.
diff --git a/drivers/gpu/drm/ms91xx/Makefile b/drivers/gpu/drm/ms91xx/Makefile
new file mode 100644
index 0000000000000..98dac2602c135
--- /dev/null
+++ b/drivers/gpu/drm/ms91xx/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+ms91xx-y := \
+	ms91xx_registers.o \
+	ms91xx_connector.o \
+	ms91xx_transfer.o \
+	ms91xx_drv.o
+
+obj-$(CONFIG_DRM_MS91XX) := ms91xx.o
-- 
2.55.0

Re: [PATCH v1 2/4] drm/ms91xx: Add register access and connector support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] drm/ms91xx: mismatched interlace handling in ms91xx_decode_custom_timing()
- [Medium] drm/ms91xx: unchecked block index and u16 overflow in ms91xx_read_edid()
- [Medium] drm/ms91xx: odd hactive in ms91xx_decode_custom_timing() leaks uninit memory
- [Low] drm/ms91xx: multiple modes marked DRM_MODE_TYPE_PREFERRED on connector
--

commit 557c9cb985634091a4b0753b2e8063cde49e88a5
Author: Ho Jie Feng <hjf3108@gmail.com>

drm/ms91xx: Add register access and connector support

This commit adds USB control message helpers for reading registers and flash
memory, display power and mode programming sequences, custom flash timing
decoding, and DRM connector and EDID probing for the ms91xx driver.

> diff --git a/drivers/gpu/drm/ms91xx/ms91xx_connector.c b/drivers/gpu/drm/ms91xx/ms91xx_connector.c
> new file mode 100644
> index 0000000000000..d04f818fa09f2
> --- /dev/null
> +++ b/drivers/gpu/drm/ms91xx/ms91xx_connector.c
> @@ -0,0 +1,263 @@

[ ... ]

> +static int ms91xx_read_edid(void *data, u8 *buf, unsigned int block, size_t len)
> +{
> +	struct ms91xx_device *ms91xx = data;
> +	int offset = block * EDID_LENGTH;
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < len; i++) {
> +		u16 address = MS91XX_REG_EDID_BASE + offset + i;
> +
> +		ret = ms91xx_read_byte(ms91xx, address);
> +		if (ret < 0)
> +			return ret;
> +		buf[i] = ret;
> +	}
> +	return 0;
> +}

[Severity: Medium]
Should ms91xx_read_edid() check the bounds of block before computing the
register address?

In ms91xx_connector_get_modes(), drm_edid_read_custom() calls
_drm_do_get_edid(), which reads block 0 and then requests extension blocks up
to the count advertised by the sink (up to 255 extensions).

Because ms91xx_read_edid() does not limit block, any block >= 2 advances
address past the EDID region at MS91XX_REG_EDID_BASE (0xc000) into device
registers in 0xc100..0xffff, such as MS91XX_REG_MODE_SEQUENCE_2 (0xc620),
MS912X_REG_CHIP_ID (0xf000), and MS913X_REG_CHIP_ID (0xff00).

When block >= 128 (offset >= 0x4000), MS91XX_REG_EDID_BASE + offset + i
overflows the 16-bit address variable and wraps around to 0x0000..0x3fff,
reading low registers including MS91XX_REG_MODE_SEQUENCE_0 (0x0030) and
MS91XX_REG_MODE_SEQUENCE_1 (0x0033).

In addition, edid_block_read() retries each invalid block 4 times when
ms91xx_read_edid() returns 0, and each ms91xx_read_byte() performs two
synchronous USB control transfers, so an EDID advertising 255 extensions can
trigger up to 261,120 USB control transfers while holding mode_config.mutex.

[ ... ]

> +static int ms91xx_connector_get_modes(struct drm_connector *connector)
> +{
> +	struct ms91xx_device *ms91xx = to_ms91xx(connector->dev);
> +	const struct drm_edid *edid;
> +	int ret = ms91xx_add_custom_mode(connector);
> +
> +	if (ms91xx->port_type == MS91XX_VIDEO_PORT_CVBS ||
> +	    ms91xx->port_type == MS91XX_VIDEO_PORT_CVBS_SVIDEO ||
> +	    ms91xx->port_type == MS91XX_VIDEO_PORT_UNKNOWN)
> +		return ret + ms91xx_add_cvbs_svideo_modes(connector);
> +
> +	if (ms91xx->port_type == MS91XX_VIDEO_PORT_YPBPR)
> +		return ret + ms91xx_add_ypbpr_modes(connector);
> +
> +	edid = drm_edid_read_custom(connector, ms91xx_read_edid, ms91xx);
> +	if (!edid)
> +		return ret + ms91xx_add_default_hdmi_vga_modes(connector);
> +
> +	if (drm_edid_connector_update(connector, edid) >= 0)
> +		ret += drm_edid_connector_add_modes(connector);
> +	drm_edid_free(edid);
> +	return ret;
> +}

[Severity: Low]
Can ms91xx_connector_get_modes() end up marking multiple modes on the same
connector with DRM_MODE_TYPE_PREFERRED?

In ms91xx_decode_custom_timing(), every decoded custom mode (up to two modes
when the "modify2" flash marker is present) is unconditionally assigned:

	display_mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;

When ms91xx_connector_get_modes() runs, ms91xx_add_custom_mode() adds all
decoded custom modes with DRM_MODE_TYPE_PREFERRED set, and then
ms91xx_connector_get_modes() unconditionally calls
ms91xx_add_cvbs_svideo_modes(), ms91xx_add_ypbpr_modes(),
ms91xx_add_default_hdmi_vga_modes(), or drm_edid_connector_add_modes(), each
of which also sets DRM_MODE_TYPE_PREFERRED on another mode.

Per struct drm_display_mode in include/drm/drm_modes.h, there should only be
one preferred mode per connector at any given time, and having multiple
preferred modes causes drm_mode_sort() to tie-break among them by resolution
(hdisplay * vdisplay).

[ ... ]

> diff --git a/drivers/gpu/drm/ms91xx/ms91xx_registers.c b/drivers/gpu/drm/ms91xx/ms91xx_registers.c
> new file mode 100644
> index 0000000000000..84cbdc78002b1
> --- /dev/null
> +++ b/drivers/gpu/drm/ms91xx/ms91xx_registers.c
> @@ -0,0 +1,386 @@

[ ... ]

> +static int
> +ms91xx_decode_custom_timing(struct ms91xx_custom_mode *custom_mode,
> +			    const struct ms91xx_custom_timing_record *record)
> +{
> +	struct drm_display_mode *display_mode = &custom_mode->display_mode;
> +	u16 hsyncwidth = le16_to_cpu(record->hsyncwidth);
> +	u16 vsyncwidth = le16_to_cpu(record->vsyncwidth);
> +	u16 hoffset = le16_to_cpu(record->hoffset);
> +	u16 voffset = le16_to_cpu(record->voffset);
> +	u16 hactive = le16_to_cpu(record->hactive);
> +	u16 vactive = le16_to_cpu(record->vactive);
> +	u16 htotal = le16_to_cpu(record->htotal);
> +	u16 vtotal = le16_to_cpu(record->vtotal);
> +	u16 pixclk = le16_to_cpu(record->pixclk);
> +	u16 vfreq = le16_to_cpu(record->vfreq);
> +	unsigned int hblank, vblank;
> +	int refresh;
> +
> +	if (!hactive || hactive > MS91XX_MAX_WIDTH ||
> +	    !vactive || vactive > MS91XX_MAX_HEIGHT ||
> +	    htotal <= hactive || vtotal <= vactive || !pixclk || !vfreq)
> +		return -EINVAL;

[Severity: Medium]
Should ms91xx_decode_custom_timing() reject odd hactive values (hactive % 2 != 0)?

In ms91xx_set_resolution(), the hardware is configured exclusively for packed
4:2:2 UYVY (MS91XX_PIXFMT_UYVY), which encodes horizontal pixels in pairs.

If a custom timing record in flash has an odd hactive and is selected with
fb->width == hactive, ms91xx_fb_send_rect() (added in the following patch)
computes:

	x = ALIGN_DOWN(rect->x1, 2);
	width = min_t(int, ALIGN(rect->x2, 2), fb->width) - x;

When rect->x2 == fb->width, min_t() clamps to the odd fb->width, producing an
odd width. ms91xx_fb_xrgb8888_to_yuv422() then calls
drm_fb_xrgb8888_to_uyvy(), which advances the destination buffer by width * 2
bytes per line while drm_fb_xrgb8888_to_uyvy_line() only writes
round_down(width, 2) * 2 bytes per line, leaving 2 trailing bytes of
uninitialized vmalloc_32() memory per scanline in transfer_buffer to be sent
over USB.

> +
> +	hblank = htotal - hactive;
> +	vblank = vtotal - vactive;
> +	if (!hsyncwidth || hsyncwidth > hoffset || hoffset > hblank ||
> +	    !vsyncwidth || vsyncwidth > voffset || voffset > vblank)
> +		return -EINVAL;
> +
> +	memset(display_mode, 0, sizeof(*display_mode));
> +	display_mode->clock = pixclk * 10;
> +	display_mode->hdisplay = hactive;
> +	display_mode->hsync_start = htotal - hoffset;
> +	display_mode->hsync_end = display_mode->hsync_start + hsyncwidth;
> +	display_mode->htotal = htotal;
> +	display_mode->vdisplay = vactive;
> +	display_mode->vsync_start = vtotal - voffset;
> +	display_mode->vsync_end = display_mode->vsync_start + vsyncwidth;
> +	display_mode->vtotal = vtotal;
> +	display_mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
> +	display_mode->flags =
> +		record->polarity & MS91XX_TIMING_POSITIVE_HSYNC ?
> +			DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
> +	display_mode->flags |=
> +		record->polarity & MS91XX_TIMING_POSITIVE_VSYNC ?
> +			DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
> +	if (!(record->polarity & MS91XX_TIMING_PROGRESSIVE))
> +		display_mode->flags |= DRM_MODE_FLAG_INTERLACE;

[Severity: Medium]
How are interlaced custom timings intended to be handled between
ms91xx_decode_custom_timing() and ms91xx_connector_init()?

When !(record->polarity & MS91XX_TIMING_PROGRESSIVE),
ms91xx_decode_custom_timing() sets DRM_MODE_FLAG_INTERLACE on display_mode and
stores the entry in ms91xx->custom_modes[].

However, ms91xx_connector_init() does not set
ms91xx->connector.interlace_allowed = true. When
drm_helper_probe_single_connector_modes() validates the modes added by
ms91xx_add_custom_mode(), drm_mode_validate_flag() marks any
DRM_MODE_FLAG_INTERLACE mode as MODE_NO_INTERLACE and drm_mode_prune_invalid()
deletes it from the connector mode list.

Meanwhile, the decoded entry remains in ms91xx->custom_modes[] and struct
ms91xx_mode only records width, height, hz, and mode without tracking
interlacing:

	custom_mode->mode.width = hactive;
	custom_mode->mode.height = vactive;
	custom_mode->mode.hz = refresh;
	custom_mode->mode.mode = record->vic;

As a result, when a progressive mode with the same width, height, and refresh
rate is later validated or enabled in ms91xx_get_mode(), it will match the
pruned interlaced entry in ms91xx->custom_modes[].

> +
> +	refresh = DIV_ROUND_CLOSEST(vfreq, 100);
> +	if (!refresh || drm_mode_vrefresh(display_mode) != refresh)
> +		return -EINVAL;
> +
> +	drm_mode_set_name(display_mode);
> +	custom_mode->mode.width = hactive;
> +	custom_mode->mode.height = vactive;
> +	custom_mode->mode.hz = refresh;
> +	custom_mode->mode.mode = record->vic;
> +
> +	return 0;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019154820.68659-1-hjf3108@gmail.com?part=2

Re: [PATCH v1 3/4] drm/ms91xx: Add atomic modesetting and USB framebuffer transfers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/ms91xx: lost frame damage in ms91xx_plane_atomic_update()
- [Medium] drm/ms91xx: uninitialized USB transfer data in ms91xx_fb_send_rect()
--

commit 0984404c243e6ff2131df1188521aaa6fa5626e8
Author: Ho Jie Feng <hjf3108@gmail.com>

drm/ms91xx: Add atomic modesetting and USB framebuffer transfers

This commit adds DRM core modesetting support, USB driver probing, and
double-buffered USB framebuffer transfers with damage tracking for
MacroSilicon MS91xx display adapters.

> diff --git a/drivers/gpu/drm/ms91xx/ms91xx_drv.c b/drivers/gpu/drm/ms91xx/ms91xx_drv.c
> new file mode 100644
> index 0000000000000..2ca855f2363c8
> --- /dev/null
> +++ b/drivers/gpu/drm/ms91xx/ms91xx_drv.c
> @@ -0,0 +1,518 @@

[ ... ]

> +static void ms91xx_plane_atomic_update(struct drm_plane *plane,
> +				       struct drm_atomic_commit *state)
> +{
> +	struct drm_plane_state *old_plane_state;
> +	struct drm_plane_state *new_plane_state;
> +	struct drm_crtc_state *new_crtc_state;
> +	struct drm_shadow_plane_state *shadow_plane_state;
> +	struct ms91xx_device *ms91xx = to_ms91xx(plane->dev);
> +	struct ms91xx_usb_request *current_request, *previous_request;
> +	struct drm_rect current_rect, rect;
> +
> +	old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> +	new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> +
> +	if (!new_plane_state->fb)
> +		return;
> +	new_crtc_state = drm_atomic_get_new_crtc_state(state,
> +						       new_plane_state->crtc);
> +	shadow_plane_state = to_drm_shadow_plane_state(new_plane_state);
> +
> +	/* Damage from the previous mode may not fit the new framebuffer. */
> +	if (drm_atomic_crtc_needs_modeset(new_crtc_state))
> +		ms91xx_clear_rect(&ms91xx->update_rect);
> +
> +	if (drm_atomic_helper_damage_merged(old_plane_state, new_plane_state,
> +					    &current_rect)) {
> +		current_request = &ms91xx->requests[ms91xx->current_request];
> +		previous_request = &ms91xx->requests[!ms91xx->current_request];
> +
> +		/* Latency reduction: requeue with the latest frame data. */
> +		if (cancel_work(&previous_request->work)) {
> +			complete(&previous_request->done);
> +			ms91xx_merge_rects(&current_rect, &current_rect,
> +					   &previous_request->transfer_rect);
> +			current_request = previous_request;
> +			ms91xx->current_request = !ms91xx->current_request;
> +		}
> +
> +		current_request->transfer_rect = current_rect;
> +
> +		/*
> +		 * The device double buffers, so we need to send the update
> +		 * rects of the last two frames.
> +		 */
> +		ms91xx_merge_rects(&rect, &current_rect, &ms91xx->update_rect);
> +		if (ms91xx_fb_send_rect(new_plane_state->fb,
> +					&shadow_plane_state->data[0],
> +					&shadow_plane_state->fmtcnv_state,
> +					&rect)) {
> +			/* In case of error, merge the rects to update later */
> +			ms91xx_merge_rects(&ms91xx->update_rect,
> +					   &ms91xx->update_rect, &rect);
> +		} else {
> +			ms91xx->update_rect = current_rect;
> +		}
> +	}
> +}

[Severity: Medium]
Can ms91xx_plane_atomic_update() drop damage for one of the device's two
buffers when pending work is canceled or after a failed transfer?

Consider three consecutive updates while requests[0].work is still running
usb_sg_wait() on the ordered workqueue:

1. Frame 1 (damage R1) queues requests[0].work and sets
   ms91xx->update_rect = R1.
2. Frame 2 (damage R2) sees cancel_work(&requests[0].work) return false,
   sets requests[1].transfer_rect = R2, queues requests[1].work with
   R1 U R2, and immediately overwrites ms91xx->update_rect = R2 before
   requests[1].work runs.
3. Frame 3 (damage R3) arrives while requests[0].work is still running and
   requests[1].work is still pending. cancel_work(&requests[1].work)
   succeeds, merging R3 with previous_request->transfer_rect (R2) and then
   with ms91xx->update_rect (also R2).

Because Frame 2 already overwrote ms91xx->update_rect with R2 and its
transfer was canceled, R1 is never sent in Frame 3 and the second hardware
buffer never receives R1.

Similarly, if ms91xx_fb_send_rect() fails on Frame 1 (for example, timing
out on current_request->done), R1 is merged into ms91xx->update_rect and no
USB transfer takes place. When Frame 2 (damage R2) succeeds, it sends
R1 U R2 to only one hardware buffer and then sets
ms91xx->update_rect = R2, so Frame 3 only sends R2 U R3 to the other
hardware buffer, leaving it without R1.

[ ... ]

> diff --git a/drivers/gpu/drm/ms91xx/ms91xx_transfer.c b/drivers/gpu/drm/ms91xx/ms91xx_transfer.c
> new file mode 100644
> index 0000000000000..8b88b2206ce3d
> --- /dev/null
> +++ b/drivers/gpu/drm/ms91xx/ms91xx_transfer.c
> @@ -0,0 +1,218 @@

[ ... ]

> +int ms91xx_init_request(struct ms91xx_device *ms91xx,
> +			struct ms91xx_usb_request *request, size_t len)
> +{
> +	int ret;
> +	unsigned int i, num_pages;
> +	void *data;
> +	struct page **pages;
> +	void *ptr;
> +
> +	data = vmalloc_32(len);
> +	if (!data)
> +		return -ENOMEM;

[ ... ]

> +static void ms91xx_fb_xrgb8888_to_yuv422(void *dst,
> +					 const struct iosys_map *src,
> +					 struct drm_framebuffer *fb,
> +					 const struct drm_rect *rect,
> +					 struct drm_format_conv_state *fmtcnv_state)
> +{
> +	struct ms91xx_frame_update_header *header = dst;
> +	struct iosys_map dst_map;
> +	u32 position, dimensions;
> +	int x, y1, width;
> +
> +	y1 = rect->y1;
> +	x = rect->x1;
> +	width = drm_rect_width(rect);
> +
> +	header->marker = cpu_to_be16(0xff00);
> +	position = ((x & 0xfff) << 12) | (y1 & 0xfff);
> +	dimensions = ((width & 0xfff) << 12) |
> +		     (drm_rect_height(rect) & 0xfff);
> +	put_unaligned_be24(position, header->position);
> +	put_unaligned_be24(dimensions, header->dimensions);
> +	dst += sizeof(*header);
> +
> +	iosys_map_set_vaddr(&dst_map, dst);
> +	drm_fb_xrgb8888_to_uyvy(&dst_map, NULL, src, fb, rect, fmtcnv_state);
> +	dst += width * drm_rect_height(rect) * 2;
> +
> +	memcpy(dst, ms91xx_end_of_buffer, sizeof(ms91xx_end_of_buffer));
> +}
> +
> +int ms91xx_fb_send_rect(struct drm_framebuffer *fb, const struct iosys_map *map,
> +			struct drm_format_conv_state *fmtcnv_state,
> +			struct drm_rect *rect)
> +{
> +	int ret = 0, idx;
> +	struct ms91xx_device *ms91xx = to_ms91xx(fb->dev);
> +	struct drm_device *drm = &ms91xx->drm;
> +	struct ms91xx_usb_request *current_request;
> +	int x, width;
> +
> +	/* UYVY stores pixels in pairs. Expand damage to a complete pair. */
> +	x = ALIGN_DOWN(rect->x1, 2);
> +	width = min_t(int, ALIGN(rect->x2, 2), fb->width) - x;
> +	rect->x1 = x;
> +	rect->x2 = x + width;
> +	rect->y2 = min_t(unsigned int, rect->y2, fb->height);

[Severity: Medium]
What happens in ms91xx_fb_send_rect() when userspace creates a framebuffer
with an odd fb->width (for example, fb->width = 641 scanned out at 640x480
with a 1-pixel source X offset) and a damage clip reaches the right edge of
the framebuffer?

Here, min_t(int, ALIGN(rect->x2, 2), fb->width) clamps to the odd fb->width,
leaving both width and rect->x2 odd.

When ms91xx_fb_xrgb8888_to_yuv422() then calls drm_fb_xrgb8888_to_uyvy(),
drm_fb_xrgb8888_to_uyvy_line() only converts round_down(pixels, 2) pixels per
line while drm_fb_xfrm() advances dst by width * 2 bytes per line. Because
request->transfer_buffer was allocated with vmalloc_32() in
ms91xx_init_request() without zeroing, doesn't this leave 2 bytes of
uninitialized vmalloc memory per scanline that get sent over USB?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019154820.68659-1-hjf3108@gmail.com?part=3