[PATCH v3 20/32] drm/via: Add via_pll.c

[PATCH v3 20/32] drm/via: Add via_pll.c

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Note that the code here is GPL based.

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/via_pll.c | 255 ++++++++++++++++++++++++++++++++++
 1 file changed, 255 insertions(+)
 create mode 100644 drivers/gpu/drm/via/via_pll.c

diff --git a/drivers/gpu/drm/via/via_pll.c b/drivers/gpu/drm/via/via_pll.c
new file mode 100644
index 000000000000..4ebaa5ce1b70
--- /dev/null
+++ b/drivers/gpu/drm/via/via_pll.c
@@ -0,0 +1,255 @@
+/*
+ * Copyright 2012 James Simmons. All Rights Reserved.
+ * Copyright 1998-2009 VIA Technologies, Inc. All Rights Reserved.
+ * Copyright 2001-2009 S3 Graphics, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation;
+ * either version 2, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTIES OR REPRESENTATIONS; without even
+ * the implied warranty of MERCHANTABILITY or FITNESS FOR
+ * A PARTICULAR PURPOSE.See the GNU General Public License
+ * for more details.
+ *
+ * Author(s):
+ * James Simmons <jsimmons@infradead.org>
+ */
+
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+
+#include "via_drv.h"
+
+
+#define CSR_VCO_UP	600000000
+#define CSR_VCO_DOWN	300000000
+
+#define PLL_DTZ_DEFAULT		(BIT(0) | BIT(1))
+
+#define VIA_CLK_REFERENCE	14318180
+
+struct pll_mrn_value {
+	u32 pll_m;
+	u32 pll_r;
+	u32 pll_n;
+	u32 diff_clk;
+	u32 pll_fout;
+};
+
+/*
+ * This function first gets the best frequency M, R, N value
+ * to program the PLL according to the supplied frequence
+ * passed in. After we get the MRN values the results are
+ * formatted to fit properly into the PLL clock registers.
+ *
+ * PLL registers M, R, N value
+ * [31:16]  DM[7:0]
+ * [15:8 ]  DR[2:0]
+ * [7 :0 ]  DN[6:0]
+ */
+u32 via_get_clk_value(struct drm_device *dev, u32 freq)
+{
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	u32 best_pll_n = 2, best_pll_r = 0, best_pll_m = 2, best_clk_diff = freq;
+	u32 pll_fout, pll_fvco, pll_mrn = 0;
+	u32 pll_n, pll_r, pll_m, clk_diff;
+	struct pll_mrn_value pll_tmp[5] = {
+		{ 0, 0, 0, 0, 0 },
+		{ 0, 0, 0, 0, 0 },
+		{ 0, 0, 0, 0, 0 },
+		{ 0, 0, 0, 0, 0 },
+		{ 0, 0, 0, 0, 0 } };
+	int count;
+
+	if ((pdev->device != PCI_DEVICE_ID_VIA_CLE266_GFX) &&
+		(pdev->device != PCI_DEVICE_ID_VIA_KM400_GFX)) {
+		/* DN[6:0] */
+		for (pll_n = 2; pll_n < 6; pll_n++) {
+			/* DR[2:0] */
+			for (pll_r = 0; pll_r < 6; pll_r++) {
+				/* DM[9:0] */
+				for (pll_m = 2; pll_m < 512; pll_m++) {
+					/* first divide pll_n then multiply
+					 * pll_m. We have to reduce pll_m
+					 * to 512 to get rid of the overflow */
+					pll_fvco = (VIA_CLK_REFERENCE / pll_n) * pll_m;
+					if ((pll_fvco >= CSR_VCO_DOWN) && (pll_fvco <= CSR_VCO_UP)) {
+						pll_fout = pll_fvco >> pll_r;
+						if (pll_fout < freq)
+							clk_diff = freq - pll_fout;
+						else
+							clk_diff = pll_fout - freq;
+
+						/* if frequency (which is the PLL we want
+						 * to set) > 150MHz, the MRN value we
+						 * write in register must < frequency, and
+						 * get MRN value whose M is the largeset */
+						if (freq >= 150000000) {
+							if ((clk_diff <= pll_tmp[0].diff_clk) || pll_tmp[0].pll_fout == 0) {
+								for (count = ARRAY_SIZE(pll_tmp) - 1; count >= 1; count--)
+									pll_tmp[count] = pll_tmp[count - 1];
+
+								pll_tmp[0].pll_m = pll_m;
+								pll_tmp[0].pll_r = pll_r;
+								pll_tmp[0].pll_n = pll_n;
+								pll_tmp[0].diff_clk = clk_diff;
+								pll_tmp[0].pll_fout = pll_fout;
+							}
+						}
+
+						if (clk_diff < best_clk_diff) {
+							best_clk_diff = clk_diff;
+							best_pll_m = pll_m;
+							best_pll_n = pll_n;
+							best_pll_r = pll_r;
+						}
+					} /* if pll_fvco in VCO range */
+				} /* for PLL M */
+			} /* for PLL R */
+		} /* for PLL N */
+
+		/* if frequency(which is the PLL we want to set) > 150MHz,
+		 * the MRN value we write in register must < frequency,
+		 * and get MRN value whose M is the largeset */
+		if (freq > 150000000) {
+			best_pll_m = pll_tmp[0].pll_m;
+			best_pll_r = pll_tmp[0].pll_r;
+			best_pll_n = pll_tmp[0].pll_n;
+		}
+	/* UniChrome IGP (CLE266, KM400(A), KN400, and P4M800 chipsets)
+	 * requires a different formula for calculating the PLL parameters.
+	 * The code was borrowed from OpenChrome DDX device driver UMS
+	 * (User Mode Setting) section, but was modified to not use float type
+	 * variables. */
+	} else {
+		for (pll_r = 0; pll_r < 4; ++pll_r) {
+			for (pll_n = (pll_r == 0) ? 2 : 1; pll_n <= 7; ++pll_n) {
+				for (pll_m = 1; pll_m <= 127; ++pll_m) {
+					pll_fout = VIA_CLK_REFERENCE * pll_m;
+					pll_fout /= (pll_n << pll_r);
+					if (pll_fout < freq)
+						clk_diff = freq - pll_fout;
+					else
+						clk_diff = pll_fout - freq;
+
+					if (clk_diff < best_clk_diff) {
+						best_clk_diff = clk_diff;
+						best_pll_m = pll_m & 0x7F;
+						best_pll_n = pll_n & 0x1F;
+						best_pll_r = pll_r & 0x03;
+					}
+				}
+			}
+		}
+	}
+
+	switch (pdev->device) {
+	case PCI_DEVICE_ID_VIA_CLE266_GFX:
+	case PCI_DEVICE_ID_VIA_KM400_GFX:
+		/* Clock Synthesizer Value 0[7:6]: DR[1:0]
+		 * Clock Synthesizer Value 0[5:0]: DN[5:0] */
+		pll_mrn = ((best_pll_r & 0x3) << 14 |
+				(best_pll_n & 0x1F) << 8);
+		/* Clock Synthesizer Value 1[6:0]: DM[6:0] */
+		pll_mrn |= (best_pll_m & 0x7F);
+		break;
+	case PCI_DEVICE_ID_VIA_CHROME9_HCM:
+	case PCI_DEVICE_ID_VIA_CHROME9_HD:
+		/* Clock Synthesizer Value 0 : DM[7:0] */
+		pll_mrn = (best_pll_m & 0xFF) << 16;
+		/* Clock Synthesizer Value 1[1:0] : DM[9:8]
+		 * Clock Synthesizer Value 1[4:2] : DR[2:0]
+		 * Clock Synthesizer Value 1[7] : DTZ[0] */
+		pll_mrn |= (((PLL_DTZ_DEFAULT & 0x1) << 7) |
+				((best_pll_r & 0x7) << 2) |
+				(((best_pll_m) >> 8) & 0x3)) << 8;
+		/* Clock Synthesizer Value 2[6:0] : DN[6:0]
+		 * Clock Synthesizer Value 2[7] : DTZ[1] */
+		pll_mrn |= (((PLL_DTZ_DEFAULT >> 1) & 0x1) << 7) |
+				((best_pll_n) & 0x7F);
+		break;
+	default:
+		/* Clock Synthesizer Value 0 : DM[7:0] */
+		pll_mrn = ((best_pll_m - 2) & 0xFF) << 16;
+		/* Clock Synthesizer Value 1[1:0] : DM[9:8]
+		 * Clock Synthesizer Value 1[4:2] : DR[2:0]
+		 * Clock Synthesizer Value 1[7] : DTZ[0] */
+		pll_mrn |= (((PLL_DTZ_DEFAULT & 0x1) << 7) |
+				((best_pll_r & 0x7) << 2) |
+				(((best_pll_m - 2) >> 8) & 0x3)) << 8;
+		/* Clock Synthesizer Value 2[6:0] : DN[6:0]
+		 * Clock Synthesizer Value 2[7] : DTZ[1] */
+		pll_mrn |= (((PLL_DTZ_DEFAULT >> 1) & 0x1) << 7) |
+				((best_pll_n - 2) & 0x7F);
+		break;
+	}
+	return pll_mrn;
+}
+
+/* Set VCLK */
+void via_set_vclock(struct drm_crtc *crtc, u32 clk)
+{
+	struct via_crtc *iga = container_of(crtc, struct via_crtc, base);
+	struct drm_device *dev = crtc->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	unsigned long max_loop = 50, i = 0;
+
+	if (!iga->index) {
+		/* IGA1 HW Reset Enable */
+		svga_wcrt_mask(VGABASE, 0x17, 0x00, BIT(7));
+
+		/* set clk */
+		if ((pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) ||
+			(pdev->device == PCI_DEVICE_ID_VIA_KM400_GFX)) {
+			vga_wseq(VGABASE, 0x46, (clk & 0xFF00) >> 8);	/* rshift + divisor */
+			vga_wseq(VGABASE, 0x47, (clk & 0x00FF));	/* multiplier */
+		} else {
+			vga_wseq(VGABASE, 0x44, (clk & 0xFF0000) >> 16);
+			vga_wseq(VGABASE, 0x45, (clk & 0x00FF00) >> 8);
+			vga_wseq(VGABASE, 0x46, (clk & 0x0000FF));
+		}
+		/* Fire */
+		svga_wmisc_mask(VGABASE, BIT(3) | BIT(2), BIT(3) | BIT(2));
+
+		/* reset pll */
+		svga_wseq_mask(VGABASE, 0x40, 0x02, 0x02);
+		svga_wseq_mask(VGABASE, 0x40, 0x00, 0x02);
+
+		/* exit hw reset */
+		while ((vga_rseq(VGABASE, 0x3C) & BIT(3)) == 0 && i++ < max_loop)
+			udelay(20);
+
+		/* IGA1 HW Reset Disable */
+		svga_wcrt_mask(VGABASE, 0x17, BIT(7), BIT(7));
+	} else {
+		/* IGA2 HW Reset Enable */
+		svga_wcrt_mask(VGABASE, 0x6A, 0x00, BIT(6));
+
+		/* set clk */
+		if ((pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) ||
+			(pdev->device == PCI_DEVICE_ID_VIA_KM400_GFX)) {
+			vga_wseq(VGABASE, 0x44, (clk & 0xFF00) >> 8);
+			vga_wseq(VGABASE, 0x45, (clk & 0x00FF));
+		} else {
+			vga_wseq(VGABASE, 0x4A, (clk & 0xFF0000) >> 16);
+			vga_wseq(VGABASE, 0x4B, (clk & 0x00FF00) >> 8);
+			vga_wseq(VGABASE, 0x4C, (clk & 0x0000FF));
+		}
+
+		/* reset pll */
+		svga_wseq_mask(VGABASE, 0x40, 0x04, 0x04);
+		svga_wseq_mask(VGABASE, 0x40, 0x00, 0x04);
+
+		/* exit hw reset */
+		while ((vga_rseq(VGABASE, 0x3C) & BIT(2)) == 0 && i++ < max_loop)
+			udelay(20);
+
+		/* IGA2 HW Reset Disble, CR6A[6] = 1 */
+		svga_wcrt_mask(VGABASE, 0x6A, BIT(6), BIT(6));
+	}
+}
--
2.35.1

[PATCH v3 21/32] drm/via: Add via_pm.c

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/via_pm.c | 187 +++++++++++++++++++++++++++++++++++
 1 file changed, 187 insertions(+)
 create mode 100644 drivers/gpu/drm/via/via_pm.c

diff --git a/drivers/gpu/drm/via/via_pm.c b/drivers/gpu/drm/via/via_pm.c
new file mode 100644
index 000000000000..5fad01447eed
--- /dev/null
+++ b/drivers/gpu/drm/via/via_pm.c
@@ -0,0 +1,187 @@
+/*
+ * Copyright © 2017-2020 Kevin Brace.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHOR(S) OR COPYRIGHT HOLDER(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author(s):
+ * Kevin Brace <kevinbrace@bracecomputerlab.com>
+ */
+
+
+#include <linux/console.h>
+#include <linux/pci.h>
+
+#include "via_drv.h"
+
+
+int via_dev_pm_ops_suspend(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(drm_dev);
+	int ret = 0;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	console_lock();
+
+	/*
+	 * Frame Buffer Size Control register (SR14) and GTI registers
+	 * (SR66 through SR6F) need to be saved and restored upon standby
+	 * resume or can lead to a display corruption issue. These registers
+	 * are only available on VX800, VX855, and VX900 chipsets. This bug
+	 * was observed on VIA Embedded EPIA-M830 mainboard.
+	 */
+	if ((pdev->device == PCI_DEVICE_ID_VIA_CHROME9_HC3) ||
+		(pdev->device == PCI_DEVICE_ID_VIA_CHROME9_HCM) ||
+		(pdev->device == PCI_DEVICE_ID_VIA_CHROME9_HD)) {
+		dev_priv->saved_sr14 = vga_rseq(VGABASE, 0x14);
+
+		dev_priv->saved_sr66 = vga_rseq(VGABASE, 0x66);
+		dev_priv->saved_sr67 = vga_rseq(VGABASE, 0x67);
+		dev_priv->saved_sr68 = vga_rseq(VGABASE, 0x68);
+		dev_priv->saved_sr69 = vga_rseq(VGABASE, 0x69);
+		dev_priv->saved_sr6a = vga_rseq(VGABASE, 0x6a);
+		dev_priv->saved_sr6b = vga_rseq(VGABASE, 0x6b);
+		dev_priv->saved_sr6c = vga_rseq(VGABASE, 0x6c);
+		dev_priv->saved_sr6d = vga_rseq(VGABASE, 0x6d);
+		dev_priv->saved_sr6e = vga_rseq(VGABASE, 0x6e);
+		dev_priv->saved_sr6f = vga_rseq(VGABASE, 0x6f);
+	}
+
+	/*
+	 * 3X5.3B through 3X5.3F are scratch pad registers.
+	 * They are important for FP detection.
+	 * Their values need to be saved because they get lost
+	 * when resuming from standby.
+	 */
+	dev_priv->saved_cr3b = vga_rcrt(VGABASE, 0x3b);
+	dev_priv->saved_cr3c = vga_rcrt(VGABASE, 0x3c);
+	dev_priv->saved_cr3d = vga_rcrt(VGABASE, 0x3d);
+	dev_priv->saved_cr3e = vga_rcrt(VGABASE, 0x3e);
+	dev_priv->saved_cr3f = vga_rcrt(VGABASE, 0x3f);
+
+	console_unlock();
+
+	ret = drm_mode_config_helper_suspend(drm_dev);
+	if (ret) {
+		DRM_ERROR("Failed to prepare for suspend.\n");
+		goto exit;
+	}
+
+	pci_save_state(pdev);
+	pci_disable_device(pdev);
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+int via_dev_pm_ops_resume(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(drm_dev);
+	void __iomem *regs = ioport_map(0x3c0, 100);
+	u8 val;
+	int ret = 0;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (pci_enable_device(pdev)) {
+		DRM_ERROR("Failed to initialize a PCI "
+				"after resume.\n");
+		ret = -EIO;
+		goto exit;
+	}
+
+	console_lock();
+
+	val = ioread8(regs + 0x03);
+	iowrite8(val | 0x1, regs + 0x03);
+	val = ioread8(regs + 0x0C);
+	iowrite8(val | 0x1, regs + 0x02);
+
+	/*
+	 * Unlock Extended IO Space.
+	 */
+	iowrite8(0x10, regs + 0x04);
+	iowrite8(0x01, regs + 0x05);
+
+	/*
+	 * Unlock CRTC register protect.
+	 */
+	iowrite8(0x47, regs + 0x14);
+
+	/*
+	 * Enable MMIO.
+	 */
+	iowrite8(0x1a, regs + 0x04);
+	val = ioread8(regs + 0x05);
+	iowrite8(val | 0x38, regs + 0x05);
+
+	/*
+	 * Frame Buffer Size Control register (SR14) and GTI registers
+	 * (SR66 through SR6F) need to be saved and restored upon standby
+	 * resume or can lead to a display corruption issue. These registers
+	 * are only available on VX800, VX855, and VX900 chipsets. This bug
+	 * was observed on VIA Embedded EPIA-M830 mainboard.
+	 */
+	if ((pdev->device == PCI_DEVICE_ID_VIA_CHROME9_HC3) ||
+		(pdev->device == PCI_DEVICE_ID_VIA_CHROME9_HCM) ||
+		(pdev->device == PCI_DEVICE_ID_VIA_CHROME9_HD)) {
+		vga_wseq(VGABASE, 0x14, dev_priv->saved_sr14);
+
+		vga_wseq(VGABASE, 0x66, dev_priv->saved_sr66);
+		vga_wseq(VGABASE, 0x67, dev_priv->saved_sr67);
+		vga_wseq(VGABASE, 0x68, dev_priv->saved_sr68);
+		vga_wseq(VGABASE, 0x69, dev_priv->saved_sr69);
+		vga_wseq(VGABASE, 0x6a, dev_priv->saved_sr6a);
+		vga_wseq(VGABASE, 0x6b, dev_priv->saved_sr6b);
+		vga_wseq(VGABASE, 0x6c, dev_priv->saved_sr6c);
+		vga_wseq(VGABASE, 0x6d, dev_priv->saved_sr6d);
+		vga_wseq(VGABASE, 0x6e, dev_priv->saved_sr6e);
+		vga_wseq(VGABASE, 0x6f, dev_priv->saved_sr6f);
+	}
+
+	/*
+	 * 3X5.3B through 3X5.3F are scratch pad registers.
+	 * They are important for FP detection.
+	 * Their values need to be restored because they are undefined
+	 * after resuming from standby.
+	 */
+	vga_wcrt(VGABASE, 0x3b, dev_priv->saved_cr3b);
+	vga_wcrt(VGABASE, 0x3c, dev_priv->saved_cr3c);
+	vga_wcrt(VGABASE, 0x3d, dev_priv->saved_cr3d);
+	vga_wcrt(VGABASE, 0x3e, dev_priv->saved_cr3e);
+	vga_wcrt(VGABASE, 0x3f, dev_priv->saved_cr3f);
+
+	console_unlock();
+
+	ret = drm_mode_config_helper_resume(drm_dev);
+	if (ret) {
+		DRM_ERROR("Failed to perform a modeset "
+				"after resume.\n");
+		goto exit;
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
--
2.35.1

[PATCH v3 22/32] drm/via: Add via_sii164.c

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/via_sii164.c | 578 +++++++++++++++++++++++++++++++
 1 file changed, 578 insertions(+)
 create mode 100644 drivers/gpu/drm/via/via_sii164.c

diff --git a/drivers/gpu/drm/via/via_sii164.c b/drivers/gpu/drm/via/via_sii164.c
new file mode 100644
index 000000000000..05a4c0230fd7
--- /dev/null
+++ b/drivers/gpu/drm/via/via_sii164.c
@@ -0,0 +1,578 @@
+/*
+ * Copyright © 2016-2018 Kevin Brace.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author(s):
+ * Kevin Brace <kevinbrace@bracecomputerlab.com>
+ */
+
+#include <linux/pci.h>
+
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_probe_helper.h>
+
+#include "via_drv.h"
+
+
+#define SII164_VEN		BIT(5)
+#define SII164_HEN		BIT(4)
+#define SII164_DSEL		BIT(3)
+#define SII164_BSEL		BIT(2)
+#define SII164_EDGE		BIT(1)
+#define SII164_PDB		BIT(0)
+
+
+static void via_sii164_power(struct i2c_adapter *i2c_bus, bool power_state)
+{
+	u8 buf;
+	u8 power_bit;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_i2c_readbytes(i2c_bus, 0x38, 0x08, &buf, 1);
+	power_bit = power_state ? SII164_PDB : 0x00;
+	buf &= ~power_bit;
+	buf |= power_bit;
+	via_i2c_writebytes(i2c_bus, 0x38, 0x08, &buf, 1);
+	DRM_DEBUG_KMS("SiI 164 (DVI) Power: %s\n",
+			power_state ? "On" : "Off");
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+
+static bool via_sii164_sense(struct i2c_adapter *i2c_bus)
+{
+	u8 buf;
+	bool rx_detected = false;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_i2c_readbytes(i2c_bus, 0x38, 0x09, &buf, 1);
+	if (buf & BIT(2)) {
+		rx_detected = true;
+	}
+
+	DRM_DEBUG_KMS("SiI 164 (DVI) Connector Sense: %s\n",
+			rx_detected ? "Connected" : "Not Connected");
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return rx_detected;
+}
+
+static void via_sii164_display_registers(struct i2c_adapter *i2c_bus)
+{
+	uint8_t i;
+	u8 buf;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	DRM_DEBUG_KMS("SiI 164 Registers:\n");
+	for (i = 0; i < 0x10; i++) {
+		via_i2c_readbytes(i2c_bus, 0x38, i, &buf, 1);
+		DRM_DEBUG_KMS("0x%02x: 0x%02x\n", i, buf);
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_sii164_init_registers(struct i2c_adapter *i2c_bus)
+{
+	u8 buf;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	buf = SII164_VEN | SII164_HEN |
+		SII164_DSEL |
+		SII164_EDGE | SII164_PDB;
+	via_i2c_writebytes(i2c_bus, 0x38, 0x08, &buf, 1);
+
+	/*
+	 * Route receiver detect bit (Offset 0x09[2]) as the output
+	 * of MSEN pin.
+	 */
+	buf = BIT(5);
+	via_i2c_writebytes(i2c_bus, 0x38, 0x09, &buf, 1);
+
+	buf = 0x90;
+	via_i2c_writebytes(i2c_bus, 0x38, 0x0a, &buf, 1);
+
+	buf = 0x89;
+	via_i2c_writebytes(i2c_bus, 0x38, 0x0c, &buf, 1);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+
+static const struct drm_encoder_funcs via_sii164_drm_encoder_funcs = {
+	.destroy = via_encoder_cleanup,
+};
+
+static void via_sii164_dpms(struct drm_encoder *encoder, int mode)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_sii164_display_registers(i2c_bus);
+	switch (mode) {
+	case DRM_MODE_DPMS_ON:
+		via_sii164_power(i2c_bus, true);
+		via_transmitter_io_pad_state(dev_priv, enc->di_port, true);
+		break;
+	case DRM_MODE_DPMS_STANDBY:
+	case DRM_MODE_DPMS_SUSPEND:
+	case DRM_MODE_DPMS_OFF:
+		via_sii164_power(i2c_bus, false);
+		via_transmitter_io_pad_state(dev_priv, enc->di_port, false);
+		break;
+	default:
+		DRM_ERROR("Bad DPMS mode.");
+		break;
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static bool via_sii164_mode_fixup(struct drm_encoder *encoder,
+				const struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	drm_mode_set_crtcinfo(adjusted_mode, 0);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return true;
+}
+
+static void via_sii164_mode_set(struct drm_encoder *encoder,
+				struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode)
+{
+	struct via_crtc *iga = container_of(encoder->crtc, struct via_crtc, base);
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_transmitter_clock_drive_strength(dev_priv, enc->di_port, 0x03);
+	via_transmitter_data_drive_strength(dev_priv, enc->di_port, 0x03);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, true);
+	if (pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) {
+		via_clock_source(dev_priv, enc->di_port, true);
+	}
+
+	via_sii164_display_registers(i2c_bus);
+	via_sii164_init_registers(i2c_bus);
+	via_sii164_display_registers(i2c_bus);
+
+	via_transmitter_display_source(dev_priv, enc->di_port, iga->index);
+exit:
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_sii164_prepare(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_sii164_power(i2c_bus, false);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, false);
+	if (pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) {
+		via_output_enable(dev_priv, enc->di_port, false);
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_sii164_commit(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_sii164_power(i2c_bus, true);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, true);
+	if (pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) {
+		via_output_enable(dev_priv, enc->di_port, true);
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_sii164_disable(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_sii164_power(i2c_bus, false);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, false);
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+
+static const struct drm_encoder_helper_funcs
+via_sii164_drm_encoder_helper_funcs = {
+	.dpms = via_sii164_dpms,
+	.mode_fixup = via_sii164_mode_fixup,
+	.mode_set = via_sii164_mode_set,
+	.prepare = via_sii164_prepare,
+	.commit = via_sii164_commit,
+	.disable = via_sii164_disable,
+};
+
+
+static enum drm_connector_status via_sii164_detect(
+					struct drm_connector *connector,
+					bool force)
+{
+	struct via_connector *con = container_of(connector,
+					struct via_connector, base);
+	struct i2c_adapter *i2c_bus;
+	enum drm_connector_status ret = connector_status_disconnected;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (con->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (con->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	if (via_sii164_sense(i2c_bus)) {
+		ret = connector_status_connected;
+		DRM_DEBUG_KMS("DVI detected.\n");
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+static const struct drm_connector_funcs via_sii164_drm_connector_funcs = {
+	.dpms = drm_helper_connector_dpms,
+	.detect = via_sii164_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = via_connector_destroy,
+	.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 via_sii164_mode_valid(struct drm_connector *connector,
+					struct drm_display_mode *mode)
+{
+	struct via_connector *con = container_of(connector,
+					struct via_connector, base);
+	struct i2c_adapter *i2c_bus;
+	u8 buf;
+	uint32_t low_freq_limit, high_freq_limit;
+	int ret;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (con->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (con->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		ret = MODE_ERROR;
+		goto exit;
+	}
+
+	via_i2c_readbytes(i2c_bus, 0x38, 0x06, &buf, 1);
+	low_freq_limit = buf * 1000;
+	via_i2c_readbytes(i2c_bus, 0x38, 0x07, &buf, 1);
+	high_freq_limit = (buf + 65) * 1000;
+	DRM_DEBUG_KMS("Low Frequency Limit: %u KHz\n", low_freq_limit);
+	DRM_DEBUG_KMS("High Frequency Limit: %u KHz\n", high_freq_limit);
+
+	if (mode->clock < low_freq_limit) {
+		ret = MODE_CLOCK_LOW;
+		goto exit;
+	}
+
+	if (mode->clock > high_freq_limit) {
+		ret = MODE_CLOCK_HIGH;
+		goto exit;
+	}
+
+	ret = MODE_OK;
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+static int via_sii164_get_modes(struct drm_connector *connector)
+{
+	struct via_connector *con = container_of(connector,
+					struct via_connector, base);
+	int count = 0;
+	struct i2c_adapter *i2c_bus;
+	struct edid *edid = NULL;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (con->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (con->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	edid = drm_get_edid(&con->base, i2c_bus);
+	if (edid) {
+		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
+			drm_connector_update_edid_property(connector, edid);
+			count = drm_add_edid_modes(connector, edid);
+			DRM_DEBUG_KMS("DVI EDID information was obtained.\n");
+		}
+
+		kfree(edid);
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs
+via_sii164_drm_connector_helper_funcs = {
+	.mode_valid = via_sii164_mode_valid,
+	.get_modes = via_sii164_get_modes,
+};
+
+bool via_sii164_probe(struct i2c_adapter *i2c_bus)
+{
+	u8 buf;
+	u16 vendor_id, device_id, revision;
+	bool device_detected = false;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_i2c_readbytes(i2c_bus, 0x38, 0x00, &buf, 1);
+	vendor_id = buf;
+	via_i2c_readbytes(i2c_bus, 0x38, 0x01, &buf, 1);
+	vendor_id |= (buf << 8);
+	DRM_DEBUG_KMS("Vendor ID: %x\n", vendor_id);
+	via_i2c_readbytes(i2c_bus, 0x38, 0x02, &buf, 1);
+	device_id = buf;
+	via_i2c_readbytes(i2c_bus, 0x38, 0x03, &buf, 1);
+	device_id |= (buf << 8);
+	DRM_DEBUG_KMS("Device ID: %x\n", device_id);
+	via_i2c_readbytes(i2c_bus, 0x38, 0x04, &buf, 1);
+	revision = buf;
+	DRM_DEBUG_KMS("Revision: %x\n", revision);
+
+	if ((vendor_id != 0x0001) || (device_id != 0x0006)) {
+		goto exit;
+	}
+
+	device_detected = true;
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return device_detected;
+}
+
+void via_sii164_init(struct drm_device *dev)
+{
+	struct via_connector *con;
+	struct via_encoder *enc;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (!dev_priv->ext_tmds_presence) {
+		goto exit;
+	}
+
+	enc = kzalloc(sizeof(*enc) + sizeof(*con), GFP_KERNEL);
+	if (!enc) {
+		DRM_ERROR("Failed to allocate connector "
+				"and encoder.\n");
+		goto exit;
+	}
+
+	drm_encoder_init(dev, &enc->base, &via_sii164_drm_encoder_funcs,
+						DRM_MODE_ENCODER_TMDS, NULL);
+	drm_encoder_helper_add(&enc->base,
+					&via_sii164_drm_encoder_helper_funcs);
+
+	enc->base.possible_crtcs = BIT(1) | BIT(0);
+	enc->base.possible_clones = 0;
+
+	enc->i2c_bus = dev_priv->ext_tmds_i2c_bus;
+	enc->di_port = dev_priv->ext_tmds_di_port;
+
+	/* Increment the number of DVI connectors. */
+	dev_priv->number_dvi++;
+
+
+	con = &enc->cons[0];
+
+	drm_connector_init(dev, &con->base, &via_sii164_drm_connector_funcs,
+				DRM_MODE_CONNECTOR_DVID);
+	drm_connector_helper_add(&con->base,
+				&via_sii164_drm_connector_helper_funcs);
+	drm_connector_register(&con->base);
+
+	con->base.doublescan_allowed = false;
+	con->base.interlace_allowed = false;
+
+	con->i2c_bus = dev_priv->ext_tmds_i2c_bus;
+
+	INIT_LIST_HEAD(&con->props);
+	drm_connector_attach_encoder(&con->base, &enc->base);
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
--
2.35.1

[PATCH v3 23/32] drm/via: Add via_tmds.c

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/via_tmds.c | 712 +++++++++++++++++++++++++++++++++
 1 file changed, 712 insertions(+)
 create mode 100644 drivers/gpu/drm/via/via_tmds.c

diff --git a/drivers/gpu/drm/via/via_tmds.c b/drivers/gpu/drm/via/via_tmds.c
new file mode 100644
index 000000000000..71dad264a46b
--- /dev/null
+++ b/drivers/gpu/drm/via/via_tmds.c
@@ -0,0 +1,712 @@
+/*
+ * Copyright © 2016-2018 Kevin Brace.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author(s):
+ * Kevin Brace <kevinbrace@bracecomputerlab.com>
+ */
+
+#include <linux/pci.h>
+
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_probe_helper.h>
+
+#include "via_drv.h"
+
+
+static void via_tmds_power(struct via_drm_priv *dev_priv,
+				bool power_state)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (power_state) {
+		via_lvds1_set_soft_display_period(VGABASE, true);
+		via_lvds1_set_soft_data(VGABASE, true);
+		via_tmds_set_power(VGABASE, true);
+	} else {
+		via_tmds_set_power(VGABASE, false);
+		via_lvds1_set_soft_data(VGABASE, false);
+		via_lvds1_set_soft_display_period(VGABASE, false);
+	}
+
+	DRM_INFO("DVI Power: %s\n",
+			power_state ? "On" : "Off");
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_tmds_io_pad_setting(struct via_drm_priv *dev_priv,
+					u32 di_port, bool io_pad_on)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	switch(di_port) {
+	case VIA_DI_PORT_TMDS:
+		via_lvds1_set_io_pad_setting(VGABASE,
+				io_pad_on ? 0x03 : 0x00);
+		break;
+	default:
+		break;
+	}
+
+	DRM_DEBUG_KMS("DVI I/O Pad: %s\n", io_pad_on ? "On": "Off");
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+/*
+ * Initializes most registers related to VIA Technologies IGP
+ * integrated TMDS transmitter. Synchronization polarity and
+ * display output source need to be set separately.
+ */
+static void via_tmds_init_reg(struct via_drm_priv *dev_priv)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	/* Turn off hardware controlled FP power on / off circuit. */
+	via_lvds_set_primary_hard_power(VGABASE, false);
+
+	/* Use software FP power sequence control. */
+	via_lvds_set_primary_power_seq_type(VGABASE, false);
+
+	/* Turn off software controlled primary FP power rails. */
+	via_lvds_set_primary_soft_vdd(VGABASE, false);
+	via_lvds_set_primary_soft_vee(VGABASE, false);
+
+	/* Turn off software controlled primary FP back light
+	* control. */
+	via_lvds_set_primary_soft_back_light(VGABASE, false);
+
+	/* Turn off direct control of FP back light. */
+	via_lvds_set_primary_direct_back_light_ctrl(VGABASE, false);
+
+	/* Activate DVI + LVDS2 mode. */
+	/* 3X5.D2[5:4] - Display Channel Select
+	 *               00: LVDS1 + LVDS2
+	 *               01: DVI + LVDS2
+	 *               10: One Dual LVDS Channel (High Resolution Pannel)
+	 *               11: Single Channel DVI */
+	svga_wcrt_mask(VGABASE, 0xd2, 0x10, 0x30);
+
+	/* Various DVI PLL settings should be set to default settings. */
+	/* 3X5.D1[7]   - PLL2 Reference Clock Edge Select Bit
+	 *               0: PLLCK lock to rising edge of reference clock
+	 *               1: PLLCK lock to falling edge of reference clock
+	 * 3X5.D1[6:5] - PLL2 Charge Pump Current Set Bits
+	 *               00: ICH = 12.5 uA
+	 *               01: ICH = 25.0 uA
+	 *               10: ICH = 37.5 uA
+	 *               11: ICH = 50.0 uA
+	 * 3X5.D1[4:1] - Reserved
+	 * 3X5.D1[0]   - PLL2 Control Voltage Measurement Enable Bit */
+	svga_wcrt_mask(VGABASE, 0xd1, 0x00, 0xe1);
+
+	/* Disable DVI test mode. */
+	/* 3X5.D5[7] - PD1 Enable Selection
+	 *             1: Select by power flag
+	 *             0: By register
+	 * 3X5.D5[5] - DVI Testing Mode Enable
+	 * 3X5.D5[4] - DVI Testing Format Selection
+	 *             0: Half cycle
+	 *             1: LFSR mode */
+	svga_wcrt_mask(VGABASE, 0xd5, 0x00, 0xb0);
+
+	/* Disable DVI sense interrupt. */
+	/* 3C5.2B[7] - DVI Sense Interrupt Enable
+	 *             0: Disable
+	 *             1: Enable */
+	svga_wseq_mask(VGABASE, 0x2b, 0x00, 0x80);
+
+	/* Clear DVI sense interrupt status. */
+	/* 3C5.2B[6] - DVI Sense Interrupt Status
+	 *             (This bit has a RW1C attribute.) */
+	svga_wseq_mask(VGABASE, 0x2b, 0x40, 0x40);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+/*
+ * Set TMDS (DVI) sync polarity.
+ */
+static void via_tmds_sync_polarity(struct via_drm_priv *dev_priv,
+					unsigned int flags)
+{
+	u8 syncPolarity = 0x00;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (flags & DRM_MODE_FLAG_NHSYNC) {
+		syncPolarity |= BIT(0);
+	}
+
+	if (flags & DRM_MODE_FLAG_NVSYNC) {
+		syncPolarity |= BIT(1);
+	}
+
+	via_tmds_set_sync_polarity(VGABASE, syncPolarity);
+	DRM_INFO("TMDS (DVI) Horizontal Sync Polarity: %s\n",
+		(syncPolarity & BIT(0)) ? "-" : "+");
+	DRM_INFO("TMDS (DVI) Vertical Sync Polarity: %s\n",
+		(syncPolarity & BIT(1)) ? "-" : "+");
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+/*
+ * Sets TMDS (DVI) display source.
+ */
+static void via_tmds_display_source(struct via_drm_priv *dev_priv,
+					int index)
+{
+	u8 displaySource = index;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_tmds_set_display_source(VGABASE, displaySource & 0x01);
+	DRM_INFO("TMDS (DVI) Display Source: IGA%d\n",
+			(displaySource & 0x01) + 1);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+/*
+ * Routines for controlling stuff on the TMDS port
+ */
+static const struct drm_encoder_funcs via_tmds_enc_funcs = {
+	.destroy = via_encoder_cleanup,
+};
+
+static void via_tmds_dpms(struct drm_encoder *encoder, int mode)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	switch (mode) {
+	case DRM_MODE_DPMS_ON:
+		via_tmds_power(dev_priv, true);
+		via_tmds_io_pad_setting(dev_priv, enc->di_port, true);
+		break;
+	case DRM_MODE_DPMS_STANDBY:
+	case DRM_MODE_DPMS_SUSPEND:
+	case DRM_MODE_DPMS_OFF:
+		via_tmds_power(dev_priv, false);
+		via_tmds_io_pad_setting(dev_priv, enc->di_port, false);
+		break;
+	default:
+		DRM_ERROR("Bad DPMS mode.");
+		break;
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+/* Pass our mode to the connectors and the CRTC to give them a chance to
+ * adjust it according to limitations or connector properties, and also
+ * a chance to reject the mode entirely. Usefule for things like scaling.
+ */
+static bool via_tmds_mode_fixup(struct drm_encoder *encoder,
+				const struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode)
+{
+	drm_mode_set_crtcinfo(adjusted_mode, 0);
+	return true;
+}
+
+static void via_tmds_prepare(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_tmds_power(dev_priv, false);
+	via_tmds_io_pad_setting(dev_priv, enc->di_port, false);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_tmds_commit(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_tmds_power(dev_priv, true);
+	via_tmds_io_pad_setting(dev_priv, enc->di_port, true);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+/*
+ * Handle CX700 / VX700 and VX800 integrated TMDS (DVI) mode setting.
+ */
+static void via_tmds_mode_set(struct drm_encoder *encoder,
+				struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode)
+{
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct via_crtc *iga = container_of(encoder->crtc,
+						struct via_crtc, base);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_tmds_init_reg(dev_priv);
+	via_tmds_sync_polarity(dev_priv, adjusted_mode->flags);
+	via_tmds_display_source(dev_priv, iga->index);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_tmds_disable(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_tmds_power(dev_priv, false);
+	via_tmds_io_pad_setting(dev_priv, enc->di_port, false);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static const struct drm_encoder_helper_funcs
+via_tmds_enc_helper_funcs = {
+	.dpms = via_tmds_dpms,
+	.mode_fixup = via_tmds_mode_fixup,
+	.prepare = via_tmds_prepare,
+	.commit = via_tmds_commit,
+	.mode_set = via_tmds_mode_set,
+	.disable = via_tmds_disable,
+};
+
+static enum drm_connector_status via_tmds_detect(
+					struct drm_connector *connector,
+					bool force)
+{
+	struct via_connector *con = container_of(connector, struct via_connector, base);
+	enum drm_connector_status ret = connector_status_disconnected;
+	struct i2c_adapter *i2c_bus;
+	struct edid *edid = NULL;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else {
+		i2c_bus = NULL;
+	}
+
+	if (i2c_bus) {
+		edid = drm_get_edid(&con->base, i2c_bus);
+		if (edid) {
+			if (edid->input & DRM_EDID_INPUT_DIGITAL) {
+				drm_connector_update_edid_property(connector, edid);
+				ret = connector_status_connected;
+			}
+
+			kfree(edid);
+		}
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+static const struct drm_connector_funcs via_dvi_connector_funcs = {
+	.dpms = drm_helper_connector_dpms,
+	.detect = via_tmds_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = via_connector_destroy,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state =
+			drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state =
+			drm_atomic_helper_connector_destroy_state,
+};
+
+static enum drm_mode_status via_tmds_mode_valid(
+					struct drm_connector *connector,
+					struct drm_display_mode *mode)
+{
+	struct drm_device *dev = connector->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	int min_clock, max_clock;
+	enum drm_mode_status status = MODE_OK;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	min_clock = 25000;
+	switch (pdev->device) {
+	/* CX700(M/M2) / VX700(M/M2) Chipset */
+	case PCI_DEVICE_ID_VIA_UNICHROME_PRO_II:
+	/* VX800 / VX820 Chipset */
+	case PCI_DEVICE_ID_VIA_CHROME9_HC3:
+		max_clock = 165000;
+		break;
+	/* Illegal condition (should never get here) */
+	default:
+		max_clock = 0;
+		break;
+	}
+
+	if (mode->flags & DRM_MODE_FLAG_INTERLACE) {
+		status = MODE_NO_INTERLACE;
+		goto exit;
+	}
+
+	if (mode->flags & DRM_MODE_FLAG_DBLSCAN) {
+		status = MODE_NO_DBLESCAN;
+		goto exit;
+	}
+
+	if (mode->clock < min_clock) {
+		status = MODE_CLOCK_LOW;
+		goto exit;
+	}
+
+	if (mode->clock > max_clock) {
+		status = MODE_CLOCK_HIGH;
+		goto exit;
+	}
+
+exit:
+	DRM_DEBUG_KMS("status: %u\n", status);
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return status;
+}
+
+static int via_tmds_get_modes(struct drm_connector *connector)
+{
+	struct via_connector *con = container_of(connector, struct via_connector, base);
+	struct i2c_adapter *i2c_bus;
+	struct edid *edid = NULL;
+	int count = 0;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else {
+		i2c_bus = NULL;
+	}
+
+	if (i2c_bus) {
+		edid = drm_get_edid(&con->base, i2c_bus);
+		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
+			drm_connector_update_edid_property(connector,
+								edid);
+			count = drm_add_edid_modes(connector, edid);
+			DRM_DEBUG_KMS("DVI EDID information was obtained.\n");
+		}
+
+		kfree(edid);
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs
+via_dvi_connector_helper_funcs = {
+	.mode_valid = via_tmds_mode_valid,
+	.get_modes = via_tmds_get_modes,
+};
+
+/*
+ * Probe (pre-initialization detection) of integrated TMDS transmitters.
+ */
+void via_tmds_probe(struct drm_device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	u16 chipset = pdev->device;
+	u8 sr13, sr5a;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	/* Detect the presence of integrated TMDS transmitter. */
+	switch (chipset) {
+	case PCI_DEVICE_ID_VIA_UNICHROME_PRO_II:
+	case PCI_DEVICE_ID_VIA_CHROME9_HC3:
+		sr5a = vga_rseq(VGABASE, 0x5a);
+
+		/* Setting SR5A[0] to 1.
+		 * This allows the reading out the alternative
+		 * pin strapping information from SR12 and SR13. */
+		svga_wseq_mask(VGABASE, 0x5a, BIT(0), BIT(0));
+
+		sr13 = vga_rseq(VGABASE, 0x13);
+		DRM_DEBUG_KMS("sr13: 0x%02x\n", sr13);
+
+		vga_wseq(VGABASE, 0x5a, sr5a);
+
+		/* 3C5.13[7:6] - Integrated LVDS / DVI Mode Select
+		 *               (DVP1D15-14 pin strapping)
+		 *               00: LVDS1 + LVDS2
+		 *               01: DVI + LVDS2
+		 *               10: Dual LVDS Channel (High Resolution Panel)
+		 *               11: One DVI only (decrease the clock jitter) */
+		/* Check for DVI presence using pin strappings.
+		 * VIA Technologies NanoBook reference design based products
+		 * have their pin strappings set to a wrong setting to communicate
+		 * the presence of DVI, so it requires special handling here. */
+		if (dev_priv->is_via_nanobook) {
+			dev_priv->int_tmds_presence = true;
+			dev_priv->int_tmds_di_port = VIA_DI_PORT_TMDS;
+			dev_priv->int_tmds_i2c_bus = VIA_I2C_BUS2;
+			dev_priv->mapped_i2c_bus |= VIA_I2C_BUS2;
+			DRM_DEBUG_KMS("Integrated TMDS (DVI) "
+					"transmitter detected.\n");
+		} else if (((!(sr13 & BIT(7))) && (sr13 & BIT(6))) ||
+				((sr13 & BIT(7)) && (sr13 & BIT(6)))) {
+			dev_priv->int_tmds_presence = true;
+			dev_priv->int_tmds_di_port = VIA_DI_PORT_TMDS;
+			dev_priv->int_tmds_i2c_bus = VIA_I2C_BUS2;
+			dev_priv->mapped_i2c_bus |= VIA_I2C_BUS2;
+			DRM_DEBUG_KMS("Integrated TMDS (DVI) "
+					"transmitter detected via pin "
+					"strapping.\n");
+		} else {
+			dev_priv->int_tmds_presence = false;
+			dev_priv->int_tmds_di_port = VIA_DI_PORT_NONE;
+			dev_priv->int_tmds_i2c_bus = VIA_I2C_NONE;
+		}
+
+		break;
+	default:
+		dev_priv->int_tmds_presence = false;
+		dev_priv->int_tmds_di_port = VIA_DI_PORT_NONE;
+		dev_priv->int_tmds_i2c_bus = VIA_I2C_NONE;
+		break;
+	}
+
+	DRM_DEBUG_KMS("int_tmds_presence: %x\n",
+			dev_priv->int_tmds_presence);
+	DRM_DEBUG_KMS("int_tmds_di_port: 0x%08x\n",
+			dev_priv->int_tmds_di_port);
+	DRM_DEBUG_KMS("int_tmds_i2c_bus: 0x%08x\n",
+			dev_priv->int_tmds_i2c_bus);
+	DRM_DEBUG_KMS("mapped_i2c_bus: 0x%08x\n",
+			dev_priv->mapped_i2c_bus);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+void via_tmds_init(struct drm_device *dev)
+{
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct via_connector *con;
+	struct via_encoder *enc;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (!dev_priv->int_tmds_presence) {
+		goto exit;
+	}
+
+	enc = kzalloc(sizeof(*enc) + sizeof(*con), GFP_KERNEL);
+	if (!enc) {
+		DRM_ERROR("Failed to allocate connector "
+				"and encoder.\n");
+		goto exit;
+	}
+
+	/* Setup the encoders and attach them */
+	drm_encoder_init(dev, &enc->base, &via_tmds_enc_funcs,
+				DRM_MODE_ENCODER_TMDS, NULL);
+	drm_encoder_helper_add(&enc->base, &via_tmds_enc_helper_funcs);
+
+	enc->base.possible_crtcs = BIT(1) | BIT(0);
+	enc->base.possible_clones = 0;
+
+	enc->di_port = dev_priv->int_tmds_di_port;
+
+	/* Increment the number of DVI connectors. */
+	dev_priv->number_dvi++;
+
+
+	con = &enc->cons[0];
+	drm_connector_init(dev, &con->base, &via_dvi_connector_funcs,
+				DRM_MODE_CONNECTOR_DVID);
+	drm_connector_helper_add(&con->base, &via_dvi_connector_helper_funcs);
+	drm_connector_register(&con->base);
+
+	con->i2c_bus = dev_priv->int_tmds_i2c_bus;
+	con->base.doublescan_allowed = false;
+	con->base.interlace_allowed = true;
+	INIT_LIST_HEAD(&con->props);
+
+	drm_connector_attach_encoder(&con->base, &enc->base);
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+/*
+ * Probe (pre-initialization detection) of external DVI transmitters.
+ */
+void via_ext_dvi_probe(struct drm_device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+	u16 chipset = pdev->device;
+	u8 sr12, sr13;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	dev_priv->ext_tmds_presence = false;
+	dev_priv->ext_tmds_i2c_bus = VIA_I2C_NONE;
+	dev_priv->ext_tmds_transmitter = VIA_TMDS_NONE;
+
+	if ((!dev_priv->ext_tmds_presence) &&
+		(!(dev_priv->mapped_i2c_bus & VIA_I2C_BUS2))) {
+		i2c_bus = via_find_ddc_bus(0x31);
+		if (via_vt1632_probe(i2c_bus)) {
+			dev_priv->ext_tmds_presence = true;
+			dev_priv->ext_tmds_i2c_bus = VIA_I2C_BUS2;
+			dev_priv->ext_tmds_transmitter = VIA_TMDS_VT1632;
+			dev_priv->mapped_i2c_bus |= VIA_I2C_BUS2;
+		} else if (via_sii164_probe(i2c_bus)) {
+			dev_priv->ext_tmds_presence = true;
+			dev_priv->ext_tmds_i2c_bus = VIA_I2C_BUS2;
+			dev_priv->ext_tmds_transmitter = VIA_TMDS_SII164;
+			dev_priv->mapped_i2c_bus |= VIA_I2C_BUS2;
+		}
+	}
+
+	if ((!(dev_priv->ext_tmds_presence)) &&
+		(!(dev_priv->mapped_i2c_bus & VIA_I2C_BUS4))) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+		if (via_vt1632_probe(i2c_bus)) {
+			dev_priv->ext_tmds_presence = true;
+			dev_priv->ext_tmds_i2c_bus = VIA_I2C_BUS4;
+			dev_priv->ext_tmds_transmitter = VIA_TMDS_VT1632;
+			dev_priv->mapped_i2c_bus |= VIA_I2C_BUS4;
+		} else if (via_sii164_probe(i2c_bus)) {
+			dev_priv->ext_tmds_presence = true;
+			dev_priv->ext_tmds_i2c_bus = VIA_I2C_BUS4;
+			dev_priv->ext_tmds_transmitter = VIA_TMDS_SII164;
+			dev_priv->mapped_i2c_bus |= VIA_I2C_BUS4;
+		}
+	}
+
+	sr12 = vga_rseq(VGABASE, 0x12);
+	sr13 = vga_rseq(VGABASE, 0x13);
+	DRM_DEBUG_KMS("SR12: 0x%02x\n", sr12);
+	DRM_DEBUG_KMS("SR13: 0x%02x\n", sr13);
+
+	if (dev_priv->ext_tmds_presence) {
+		switch (chipset) {
+		case PCI_DEVICE_ID_VIA_CLE266_GFX:
+			/* 3C5.12[5] - FPD18 pin strapping (DIP0)
+			 *             0: DVI
+			 *             1: TV */
+			if (!(sr12 & BIT(5))) {
+				dev_priv->ext_tmds_di_port = VIA_DI_PORT_DIP0;
+
+			/* 3C5.12[4] - FPD17 pin strapping (DIP1)
+			 *             0: DVI / Capture
+			 *             1: Panel */
+			} else if (!(sr12 & BIT(4))) {
+				dev_priv->ext_tmds_di_port = VIA_DI_PORT_DIP1;
+			} else {
+				dev_priv->ext_tmds_di_port = VIA_DI_PORT_NONE;
+			}
+
+			break;
+		case PCI_DEVICE_ID_VIA_KM400_GFX:
+		case PCI_DEVICE_ID_VIA_K8M800_GFX:
+		case PCI_DEVICE_ID_VIA_P4M800_PRO_GFX:
+		case PCI_DEVICE_ID_VIA_PM800_GFX:
+			/*
+			 * For DVP0 to be configured to not be used for
+			 * a TV encoder, DVP0D[6] (SR12[6]) needs to be
+			 * strapped low (0).  In addition, DVP0D[5]
+			 * (SR12[5]) also needs to be strapped low (0)
+			 * for DVP0 to be configured for DVI
+			 * transmitter use.
+			 */
+			if (!(sr12 & BIT(6)) && (!(sr12 & BIT(5)))) {
+				dev_priv->ext_tmds_di_port = VIA_DI_PORT_DVP0;
+			} else {
+				dev_priv->ext_tmds_di_port = VIA_DI_PORT_NONE;
+			}
+
+			break;
+		case PCI_DEVICE_ID_VIA_P4M890_GFX:
+		case PCI_DEVICE_ID_VIA_CHROME9:
+		case PCI_DEVICE_ID_VIA_CHROME9_HC:
+			/* Assume DVP2 as DVP0. Hence, VIA_DI_PORT_DVP0
+			 * is used. */
+			/* 3C5.12[6] - DVP2D6 pin strapping
+			 *             0: Disable DVP2 (Digital Video Port 2)
+			 *             1: Enable DVP2 (Digital Video Port 2)
+			 * 3C5.12[5] - DVP2D5 pin strapping
+			 *             0: TMDS transmitter (DVI)
+			 *             1: TV encoder */
+			if ((sr12 & BIT(6)) && (!(sr12 & BIT(5)))) {
+				dev_priv->ext_tmds_di_port = VIA_DI_PORT_DVP0;
+			} else {
+				dev_priv->ext_tmds_di_port = VIA_DI_PORT_NONE;
+			}
+
+			break;
+		case PCI_DEVICE_ID_VIA_UNICHROME_PRO_II:
+		case PCI_DEVICE_ID_VIA_CHROME9_HC3:
+		case PCI_DEVICE_ID_VIA_CHROME9_HCM:
+		case PCI_DEVICE_ID_VIA_CHROME9_HD:
+			dev_priv->ext_tmds_di_port = VIA_DI_PORT_DVP1;
+			break;
+		default:
+			dev_priv->ext_tmds_di_port = VIA_DI_PORT_NONE;
+			break;
+		}
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+void via_ext_dvi_init(struct drm_device *dev)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_vt1632_init(dev);
+	via_sii164_init(dev);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
--
2.35.1

[PATCH v3 24/32] drm/via: Add via_ttm.c

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/via_ttm.c | 168 ++++++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 drivers/gpu/drm/via/via_ttm.c

diff --git a/drivers/gpu/drm/via/via_ttm.c b/drivers/gpu/drm/via/via_ttm.c
new file mode 100644
index 000000000000..8676623ee074
--- /dev/null
+++ b/drivers/gpu/drm/via/via_ttm.c
@@ -0,0 +1,168 @@
+/*
+ * Copyright © 2018-2019 Kevin Brace.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sub license,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHOR(S) OR COPYRIGHT HOLDER(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author(s):
+ * Kevin Brace <kevinbrace@bracecomputerlab.com>
+ */
+/*
+ * via_ttm.c
+ *
+ * TTM code as part of the TTM memory allocator.
+ * Currently a basic implementation with no DMA support.
+ *
+ */
+
+#include <linux/pci.h>
+
+#include <drm/ttm/ttm_bo_api.h>
+#include <drm/ttm/ttm_bo_driver.h>
+
+#include "via_drv.h"
+
+
+static void via_bo_move_notify(struct ttm_buffer_object *bo, bool evict,
+				struct ttm_resource *new_mem)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return;
+}
+
+static struct ttm_tt *via_ttm_tt_create(struct ttm_buffer_object *bo,
+					uint32_t page_flags)
+{
+	struct ttm_tt *tt;
+	int ret;
+
+	tt = kzalloc(sizeof(*tt), GFP_KERNEL);
+	if (!tt)
+		return NULL;
+
+	ret = ttm_tt_init(tt, bo, page_flags, ttm_cached, 0);
+	if (ret < 0)
+		goto err_ttm_tt_init;
+
+	return tt;
+
+err_ttm_tt_init:
+	kfree(tt);
+	return NULL;
+}
+
+static void via_ttm_tt_destroy(struct ttm_device *bdev, struct ttm_tt *tt)
+{
+	ttm_tt_fini(tt);
+	kfree(tt);
+}
+
+static void via_bo_evict_flags(struct ttm_buffer_object *bo,
+				struct ttm_placement *placement)
+{
+	struct via_bo *driver_bo = to_ttm_bo(bo);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (bo->destroy == &via_ttm_bo_destroy) {
+		goto exit;
+	}
+
+	switch (bo->resource->mem_type) {
+	case TTM_PL_VRAM:
+		via_ttm_domain_to_placement(driver_bo, TTM_PL_VRAM);
+		break;
+	default:
+		via_ttm_domain_to_placement(driver_bo, TTM_PL_SYSTEM);
+		break;
+	}
+
+	*placement = driver_bo->placement;
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static int via_bo_move(struct ttm_buffer_object *bo, bool evict,
+			struct ttm_operation_ctx *ctx,
+			struct ttm_resource *new_mem,
+			struct ttm_place *hop)
+{
+	int ret;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_bo_move_notify(bo, evict, new_mem);
+	ret = ttm_bo_move_memcpy(bo, ctx, new_mem);
+	if (ret) {
+		swap(*new_mem, *bo->resource);
+		via_bo_move_notify(bo, false, new_mem);
+		swap(*new_mem, *bo->resource);
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+static void via_bo_delete_mem_notify(struct ttm_buffer_object *bo)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_bo_move_notify(bo, false, NULL);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return;
+}
+
+static int via_bo_io_mem_reserve(struct ttm_device *bdev,
+					struct ttm_resource *mem)
+{
+	struct via_drm_priv *dev_priv = container_of(bdev,
+						struct via_drm_priv, bdev);
+	int ret = 0;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	switch (mem->mem_type) {
+	case TTM_PL_SYSTEM:
+		break;
+	case TTM_PL_VRAM:
+		mem->bus.offset = dev_priv->vram_start +
+					(mem->start << PAGE_SHIFT);
+		mem->bus.is_iomem = true;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+struct ttm_device_funcs via_bo_driver = {
+	.ttm_tt_create = via_ttm_tt_create,
+	.ttm_tt_destroy = via_ttm_tt_destroy,
+	.eviction_valuable = ttm_bo_eviction_valuable,
+	.evict_flags = via_bo_evict_flags,
+	.move = via_bo_move,
+	.delete_mem_notify = via_bo_delete_mem_notify,
+	.io_mem_reserve = via_bo_io_mem_reserve,
+};
--
2.35.1

[PATCH v3 25/32] drm/via: Add via_vt1632.c

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/via_vt1632.c | 598 +++++++++++++++++++++++++++++++
 1 file changed, 598 insertions(+)
 create mode 100644 drivers/gpu/drm/via/via_vt1632.c

diff --git a/drivers/gpu/drm/via/via_vt1632.c b/drivers/gpu/drm/via/via_vt1632.c
new file mode 100644
index 000000000000..30e004e421c0
--- /dev/null
+++ b/drivers/gpu/drm/via/via_vt1632.c
@@ -0,0 +1,598 @@
+/*
+ * Copyright © 2016-2018 Kevin Brace.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Author(s):
+ * Kevin Brace <kevinbrace@bracecomputerlab.com>
+ */
+
+#include <linux/pci.h>
+
+#include <drm/drm_atomic_state_helper.h>
+#include <drm/drm_probe_helper.h>
+
+#include "via_drv.h"
+
+
+#define VIA_VT1632_VEN		BIT(5)
+#define VIA_VT1632_HEN		BIT(4)
+#define VIA_VT1632_DSEL		BIT(3)
+#define VIA_VT1632_BSEL		BIT(2)
+#define VIA_VT1632_EDGE		BIT(1)
+#define VIA_VT1632_PDB		BIT(0)
+
+
+static void via_vt1632_power(struct i2c_adapter *i2c_bus, bool power_state)
+{
+	u8 buf;
+	u8 power_bit;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_i2c_readbytes(i2c_bus, 0x08, 0x08, &buf, 1);
+	power_bit = power_state ? VIA_VT1632_PDB : 0x00;
+	buf &= ~power_bit;
+	buf |= power_bit;
+	via_i2c_writebytes(i2c_bus, 0x08, 0x08, &buf, 1);
+	DRM_DEBUG_KMS("VT1632 (DVI) Power: %s\n",
+			power_state ? "On" : "Off");
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+
+static bool via_vt1632_sense(struct i2c_adapter *i2c_bus)
+{
+	u8 buf;
+	bool rx_detected = false;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_i2c_readbytes(i2c_bus, 0x08, 0x09, &buf, 1);
+	if (buf & BIT(2)) {
+		rx_detected = true;
+	}
+
+	DRM_DEBUG_KMS("VT1632 (DVI) Connector Sense: %s\n",
+			rx_detected ? "Connected" : "Not Connected");
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return rx_detected;
+}
+
+static void via_vt1632_display_registers(struct i2c_adapter *i2c_bus)
+{
+	uint8_t i;
+	u8 buf;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	DRM_DEBUG_KMS("VT1632(A) Registers:\n");
+	for (i = 0; i < 0x10; i++) {
+		via_i2c_readbytes(i2c_bus, 0x08, i, &buf, 1);
+		DRM_DEBUG_KMS("0x%02x: 0x%02x\n", i, buf);
+	}
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_vt1632_init_registers(struct i2c_adapter *i2c_bus)
+{
+	u8 buf;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	/*
+	 * For Wyse Cx0 thin client VX855 chipset DVP1 (Digital Video
+	 * Port 1), use 12-bit mode with dual edge transfer, along
+	 * with rising edge data capture first mode. This is likely
+	 * true for CX700, VX700, VX800, and VX900 chipsets as well.
+	 */
+	buf = VIA_VT1632_VEN | VIA_VT1632_HEN |
+		VIA_VT1632_DSEL |
+		VIA_VT1632_EDGE | VIA_VT1632_PDB;
+	via_i2c_writebytes(i2c_bus, 0x08, 0x08, &buf, 1);
+
+	/*
+	 * Route receiver detect bit (Offset 0x09[2]) as the output
+	 * of MSEN pin.
+	 */
+	buf = BIT(5);
+	via_i2c_writebytes(i2c_bus, 0x08, 0x09, &buf, 1);
+
+	/*
+	 * Turning on deskew feature caused screen display issues.
+	 * This was observed with Wyse Cx0.
+	 */
+	buf = 0x00;
+	via_i2c_writebytes(i2c_bus, 0x08, 0x0a, &buf, 1);
+
+	/*
+	 * While VIA Technologies VT1632A datasheet insists on setting
+	 * this register to 0x89 as the recommended setting, in
+	 * practice, this leads to a blank screen on the display with
+	 * Wyse Cx0. According to Silicon Image SiI 164 datasheet
+	 * (VT1632(A) is a pin and mostly register compatible chip),
+	 * offset 0x0C is for PLL filter enable, PLL filter setting,
+	 * and continuous SYNC enable bits. All of these are turned
+	 * off for proper operation.
+	 */
+	buf = 0x00;
+	via_i2c_writebytes(i2c_bus, 0x08, 0x0c, &buf, 1);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+
+static const struct drm_encoder_funcs via_vt1632_drm_encoder_funcs = {
+	.destroy = via_encoder_cleanup,
+};
+
+static void via_vt1632_dpms(struct drm_encoder *encoder, int mode)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_vt1632_display_registers(i2c_bus);
+	switch (mode) {
+	case DRM_MODE_DPMS_ON:
+		via_vt1632_power(i2c_bus, true);
+		via_transmitter_io_pad_state(dev_priv, enc->di_port, true);
+		break;
+	case DRM_MODE_DPMS_STANDBY:
+	case DRM_MODE_DPMS_SUSPEND:
+	case DRM_MODE_DPMS_OFF:
+		via_vt1632_power(i2c_bus, false);
+		via_transmitter_io_pad_state(dev_priv, enc->di_port, false);
+		break;
+	default:
+		DRM_ERROR("Bad DPMS mode.");
+		break;
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static bool via_vt1632_mode_fixup(struct drm_encoder *encoder,
+				const struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode)
+{
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	drm_mode_set_crtcinfo(adjusted_mode, 0);
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return true;
+}
+
+static void via_vt1632_mode_set(struct drm_encoder *encoder,
+				struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode)
+{
+	struct via_crtc *iga = container_of(encoder->crtc, struct via_crtc, base);
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_transmitter_clock_drive_strength(dev_priv, enc->di_port, 0x03);
+	via_transmitter_data_drive_strength(dev_priv, enc->di_port, 0x03);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, true);
+	if (pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) {
+		via_clock_source(dev_priv, enc->di_port, true);
+	}
+
+	via_vt1632_display_registers(i2c_bus);
+	via_vt1632_init_registers(i2c_bus);
+	via_vt1632_display_registers(i2c_bus);
+
+	via_transmitter_display_source(dev_priv, enc->di_port, iga->index);
+exit:
+
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_vt1632_prepare(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_vt1632_power(i2c_bus, false);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, false);
+	if (pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) {
+		via_output_enable(dev_priv, enc->di_port, false);
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_vt1632_commit(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_vt1632_power(i2c_bus, true);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, true);
+	if (pdev->device == PCI_DEVICE_ID_VIA_CLE266_GFX) {
+		via_output_enable(dev_priv, enc->di_port, true);
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+static void via_vt1632_disable(struct drm_encoder *encoder)
+{
+	struct via_encoder *enc = container_of(encoder,
+					struct via_encoder, base);
+	struct drm_device *dev = encoder->dev;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+	struct i2c_adapter *i2c_bus;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (enc->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (enc->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (enc->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (enc->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (enc->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	via_vt1632_power(i2c_bus, false);
+	via_transmitter_io_pad_state(dev_priv, enc->di_port, false);
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
+
+
+static const struct drm_encoder_helper_funcs
+via_vt1632_drm_encoder_helper_funcs = {
+	.dpms = via_vt1632_dpms,
+	.mode_fixup = via_vt1632_mode_fixup,
+	.mode_set = via_vt1632_mode_set,
+	.prepare = via_vt1632_prepare,
+	.commit = via_vt1632_commit,
+	.disable = via_vt1632_disable,
+};
+
+
+static enum drm_connector_status via_vt1632_detect(
+					struct drm_connector *connector,
+					bool force)
+{
+	struct via_connector *con = container_of(connector,
+					struct via_connector, base);
+	struct i2c_adapter *i2c_bus;
+	enum drm_connector_status ret = connector_status_disconnected;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (con->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (con->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	if (via_vt1632_sense(i2c_bus)) {
+		ret = connector_status_connected;
+		DRM_DEBUG_KMS("DVI detected.\n");
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+static const struct drm_connector_funcs via_vt1632_drm_connector_funcs = {
+	.dpms = drm_helper_connector_dpms,
+	.detect = via_vt1632_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = via_connector_destroy,
+	.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 via_vt1632_mode_valid(struct drm_connector *connector,
+					struct drm_display_mode *mode)
+{
+	struct via_connector *con = container_of(connector,
+					struct via_connector, base);
+	struct i2c_adapter *i2c_bus;
+	u8 buf;
+	uint32_t low_freq_limit, high_freq_limit;
+	int ret;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (con->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (con->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		ret = MODE_ERROR;
+		goto exit;
+	}
+
+	via_i2c_readbytes(i2c_bus, 0x08, 0x06, &buf, 1);
+	low_freq_limit = buf * 1000;
+	via_i2c_readbytes(i2c_bus, 0x08, 0x07, &buf, 1);
+	high_freq_limit = (buf + 65) * 1000;
+	DRM_DEBUG_KMS("Low Frequency Limit: %u KHz\n", low_freq_limit);
+	DRM_DEBUG_KMS("High Frequency Limit: %u KHz\n", high_freq_limit);
+
+	if (mode->clock < low_freq_limit) {
+		ret = MODE_CLOCK_LOW;
+		goto exit;
+	}
+
+	if (mode->clock > high_freq_limit) {
+		ret = MODE_CLOCK_HIGH;
+		goto exit;
+	}
+
+	ret = MODE_OK;
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return ret;
+}
+
+static int via_vt1632_get_modes(struct drm_connector *connector)
+{
+	struct via_connector *con = container_of(connector,
+					struct via_connector, base);
+	int count = 0;
+	struct i2c_adapter *i2c_bus;
+	struct edid *edid = NULL;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (con->i2c_bus & VIA_I2C_BUS1) {
+		i2c_bus = via_find_ddc_bus(0x26);
+	} else if (con->i2c_bus & VIA_I2C_BUS2) {
+		i2c_bus = via_find_ddc_bus(0x31);
+	} else if (con->i2c_bus & VIA_I2C_BUS3) {
+		i2c_bus = via_find_ddc_bus(0x25);
+	} else if (con->i2c_bus & VIA_I2C_BUS4) {
+		i2c_bus = via_find_ddc_bus(0x2c);
+	} else if (con->i2c_bus & VIA_I2C_BUS5) {
+		i2c_bus = via_find_ddc_bus(0x3d);
+	} else {
+		i2c_bus = NULL;
+		goto exit;
+	}
+
+	edid = drm_get_edid(&con->base, i2c_bus);
+	if (edid) {
+		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
+			drm_connector_update_edid_property(connector, edid);
+			count = drm_add_edid_modes(connector, edid);
+			DRM_DEBUG_KMS("DVI EDID information was obtained.\n");
+		}
+
+		kfree(edid);
+	}
+
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs
+via_vt1632_drm_connector_helper_funcs = {
+	.mode_valid = via_vt1632_mode_valid,
+	.get_modes = via_vt1632_get_modes,
+};
+
+bool via_vt1632_probe(struct i2c_adapter *i2c_bus)
+{
+	u8 buf;
+	u16 vendor_id, device_id, revision;
+	bool device_detected = false;
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	via_i2c_readbytes(i2c_bus, 0x08, 0x00, &buf, 1);
+	vendor_id = buf;
+	via_i2c_readbytes(i2c_bus, 0x08, 0x01, &buf, 1);
+	vendor_id |= (buf << 8);
+	DRM_DEBUG_KMS("Vendor ID: %x\n", vendor_id);
+	via_i2c_readbytes(i2c_bus, 0x08, 0x02, &buf, 1);
+	device_id = buf;
+	via_i2c_readbytes(i2c_bus, 0x08, 0x03, &buf, 1);
+	device_id |= (buf << 8);
+	DRM_DEBUG_KMS("Device ID: %x\n", device_id);
+	via_i2c_readbytes(i2c_bus, 0x08, 0x04, &buf, 1);
+	revision = buf;
+	DRM_DEBUG_KMS("Revision: %x\n", revision);
+
+	if ((vendor_id != 0x1106) || (device_id != 0x3192)) {
+		goto exit;
+	}
+
+	device_detected = true;
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+	return device_detected;
+}
+
+void via_vt1632_init(struct drm_device *dev)
+{
+	struct via_connector *con;
+	struct via_encoder *enc;
+	struct via_drm_priv *dev_priv = to_via_drm_priv(dev);
+
+	DRM_DEBUG_KMS("Entered %s.\n", __func__);
+
+	if (!dev_priv->ext_tmds_presence) {
+		goto exit;
+	}
+
+	enc = kzalloc(sizeof(*enc) + sizeof(*con), GFP_KERNEL);
+	if (!enc) {
+		DRM_ERROR("Failed to allocate connector "
+				"and encoder.\n");
+		goto exit;
+	}
+
+	drm_encoder_init(dev, &enc->base, &via_vt1632_drm_encoder_funcs,
+						DRM_MODE_ENCODER_TMDS, NULL);
+	drm_encoder_helper_add(&enc->base,
+					&via_vt1632_drm_encoder_helper_funcs);
+
+	enc->base.possible_crtcs = BIT(1) | BIT(0);
+	enc->base.possible_clones = 0;
+
+	enc->i2c_bus = dev_priv->ext_tmds_i2c_bus;
+	enc->di_port = dev_priv->ext_tmds_di_port;
+
+	/* Increment the number of DVI connectors. */
+	dev_priv->number_dvi++;
+
+
+	con = &enc->cons[0];
+
+	drm_connector_init(dev, &con->base, &via_vt1632_drm_connector_funcs,
+				DRM_MODE_CONNECTOR_DVID);
+	drm_connector_helper_add(&con->base,
+				&via_vt1632_drm_connector_helper_funcs);
+	drm_connector_register(&con->base);
+
+	con->base.doublescan_allowed = false;
+	con->base.interlace_allowed = false;
+
+	con->i2c_bus = dev_priv->ext_tmds_i2c_bus;
+
+	INIT_LIST_HEAD(&con->props);
+	drm_connector_attach_encoder(&con->base, &enc->base);
+exit:
+	DRM_DEBUG_KMS("Exiting %s.\n", __func__);
+}
--
2.35.1

[PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Changed the uAPI.

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
index a1e125d42208..e9da45ce130a 100644
--- a/include/uapi/drm/via_drm.h
+++ b/include/uapi/drm/via_drm.h
@@ -1,4 +1,5 @@
 /*
+ * Copyright © 2020 Kevin Brace
  * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
  * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
  *
@@ -16,10 +17,10 @@
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
- * DEALINGS IN THE SOFTWARE.
+ * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
+ * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
+ * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
+ * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 #ifndef _VIA_DRM_H_
 #define _VIA_DRM_H_
@@ -81,6 +82,11 @@ extern "C" {
 #define DRM_VIA_DMA_BLIT        0x0e
 #define DRM_VIA_BLIT_SYNC       0x0f

+#define	DRM_VIA_GEM_CREATE	0x10
+#define	DRM_VIA_GEM_MAP		0x11
+#define	DRM_VIA_GEM_UNMAP	0x12
+
+
 #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
 #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
 #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
@@ -97,6 +103,10 @@ extern "C" {
 #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
 #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)

+#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
+#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
+#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
+
 /* Indices into buf.Setup where various bits of state are mirrored per
  * context and per buffer.  These can be fired at the card as a unit,
  * or in a piecewise fashion as required.
@@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
 	drm_via_blitsync_t sync;
 } drm_via_dmablit_t;

+struct drm_via_gem_create {
+	uint64_t size;
+	uint32_t alignment;
+	uint32_t domain;
+	uint32_t handle;
+	uint64_t offset;
+};
+
+struct drm_via_gem_map {
+	uint32_t handle;
+	uint64_t map_offset;
+};
+
+struct drm_via_gem_unmap {
+	uint32_t handle;
+};
+
 #if defined(__cplusplus)
 }
 #endif
--
2.35.1

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Sam Ravnborg]

Hi Kevin.

On Mon, Jul 25, 2022 at 04:53:53PM -0700, Kevin Brace wrote:
> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> Changed the uAPI.
> 
> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>

It would be great to have the new extensions to the UAPI documented
using kernel-doc.
As an example see: vc4_drm.h

There are a lot of UAPI that is missing documentation, but if we do not
add it for new UAPI then this situation never improves.

Please use __u32, __u64 like you see in other drm UAPI files.

PS. If you reply to this mail, then please keep the full mail as
usually my mails to Kevin bounces (something with STARTTLS).

	Sam

> ---
>  include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
>  1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> index a1e125d42208..e9da45ce130a 100644
> --- a/include/uapi/drm/via_drm.h
> +++ b/include/uapi/drm/via_drm.h
> @@ -1,4 +1,5 @@
>  /*
> + * Copyright © 2020 Kevin Brace
>   * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
>   * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
>   *
> @@ -16,10 +17,10 @@
>   * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>   * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>   * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> - * DEALINGS IN THE SOFTWARE.
> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>   */
Do not mix license changes with other changes - and use SPDX tag if
possible for the updated license.
See other drm UAPI files for examples.


>  #ifndef _VIA_DRM_H_
>  #define _VIA_DRM_H_
> @@ -81,6 +82,11 @@ extern "C" {
>  #define DRM_VIA_DMA_BLIT        0x0e
>  #define DRM_VIA_BLIT_SYNC       0x0f
> 
> +#define	DRM_VIA_GEM_CREATE	0x10
> +#define	DRM_VIA_GEM_MAP		0x11
> +#define	DRM_VIA_GEM_UNMAP	0x12
> +
Use the same alignment as the previous lines.
> +
Drop extra empty line.

>  #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>  #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
>  #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> @@ -97,6 +103,10 @@ extern "C" {
>  #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>  #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> 
> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> +
Use same alignment as previous lines.

>  /* Indices into buf.Setup where various bits of state are mirrored per
>   * context and per buffer.  These can be fired at the card as a unit,
>   * or in a piecewise fashion as required.
> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
>  	drm_via_blitsync_t sync;
>  } drm_via_dmablit_t;
> 
> +struct drm_via_gem_create {
> +	uint64_t size;
> +	uint32_t alignment;
> +	uint32_t domain;
> +	uint32_t handle;
> +	uint64_t offset;
> +};
I do not know if this is relevant, but adding a 64 bit parameter
(offset) that is only 32 bit aligned looks like trouble to me.
I hope others that knows this better can comment here.

> +
> +struct drm_via_gem_map {
> +	uint32_t handle;
> +	uint64_t map_offset;
> +};
Same here with the alignment.

If drm_via_gem_create.offset and drm_via_gem_map.map_offset is the same
the field should have the same name - to make the API easier to use.


> +
> +struct drm_via_gem_unmap {
> +	uint32_t handle;
> +};
> +
>  #if defined(__cplusplus)
>  }
>  #endif
> --
> 2.35.1

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 5569 bytes --]

Hi

Am 26.07.22 um 19:41 schrieb Sam Ravnborg:
> Hi Kevin.
> 
> On Mon, Jul 25, 2022 at 04:53:53PM -0700, Kevin Brace wrote:
>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>
>> Changed the uAPI.
>>
>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> It would be great to have the new extensions to the UAPI documented
> using kernel-doc.
> As an example see: vc4_drm.h
> 
> There are a lot of UAPI that is missing documentation, but if we do not
> add it for new UAPI then this situation never improves.
> 
> Please use __u32, __u64 like you see in other drm UAPI files.
> 
> PS. If you reply to this mail, then please keep the full mail as
> usually my mails to Kevin bounces (something with STARTTLS).
> 
> 	Sam
> 
>> ---
>>   include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
>>   1 file changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
>> index a1e125d42208..e9da45ce130a 100644
>> --- a/include/uapi/drm/via_drm.h
>> +++ b/include/uapi/drm/via_drm.h
>> @@ -1,4 +1,5 @@
>>   /*
>> + * Copyright © 2020 Kevin Brace
>>    * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
>>    * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
>>    *
>> @@ -16,10 +17,10 @@
>>    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>    * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> - * DEALINGS IN THE SOFTWARE.
>> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
>> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
>> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
>> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>    */
> Do not mix license changes with other changes - and use SPDX tag if
> possible for the updated license.
> See other drm UAPI files for examples.
> 
> 
>>   #ifndef _VIA_DRM_H_
>>   #define _VIA_DRM_H_
>> @@ -81,6 +82,11 @@ extern "C" {
>>   #define DRM_VIA_DMA_BLIT        0x0e
>>   #define DRM_VIA_BLIT_SYNC       0x0f
>>
>> +#define	DRM_VIA_GEM_CREATE	0x10
>> +#define	DRM_VIA_GEM_MAP		0x11
>> +#define	DRM_VIA_GEM_UNMAP	0x12
>> +
> Use the same alignment as the previous lines.
>> +
> Drop extra empty line.
> 
>>   #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>>   #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
>>   #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
>> @@ -97,6 +103,10 @@ extern "C" {
>>   #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>>   #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
>>
>> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
>> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
>> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
>> +
> Use same alignment as previous lines.
> 
>>   /* Indices into buf.Setup where various bits of state are mirrored per
>>    * context and per buffer.  These can be fired at the card as a unit,
>>    * or in a piecewise fashion as required.
>> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
>>   	drm_via_blitsync_t sync;
>>   } drm_via_dmablit_t;
>>
>> +struct drm_via_gem_create {
>> +	uint64_t size;
>> +	uint32_t alignment;
>> +	uint32_t domain;
>> +	uint32_t handle;
>> +	uint64_t offset;
>> +};
> I do not know if this is relevant, but adding a 64 bit parameter
> (offset) that is only 32 bit aligned looks like trouble to me.
> I hope others that knows this better can comment here.

The compiler will leave a 4-byte gap between handle and offset. 
Structure allocation guarantees a minimal alignment of 8 bytes, so the 
field alignment will be correct. It's all dependend on architecture, 
platofrm, calling convention, but that's the rule of thumb.

Have a look at the tool 'pahole' to inspect data-structure alignment in 
object files. You'll find plenty of gaps in compiled structure.

It's still questionable to leave the gap there. Either declare it 
explicity (e.g., __u32 empty0; )  or declare the structure with 
__attribute__((__packed__)).  Personally, I'd use the former.

Best regards
Thomas

> 
>> +
>> +struct drm_via_gem_map {
>> +	uint32_t handle;
>> +	uint64_t map_offset;
>> +};
> Same here with the alignment.
> 
> If drm_via_gem_create.offset and drm_via_gem_map.map_offset is the same
> the field should have the same name - to make the API easier to use.
> 
> 
>> +
>> +struct drm_via_gem_unmap {
>> +	uint32_t handle;
>> +};
>> +
>>   #if defined(__cplusplus)
>>   }
>>   #endif
>> --
>> 2.35.1

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Dave Airlie]

On Wed, 27 Jul 2022 at 04:18, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 26.07.22 um 19:41 schrieb Sam Ravnborg:
> > Hi Kevin.
> >
> > On Mon, Jul 25, 2022 at 04:53:53PM -0700, Kevin Brace wrote:
> >> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> >>
> >> Changed the uAPI.
> >>
> >> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> >
> > It would be great to have the new extensions to the UAPI documented
> > using kernel-doc.
> > As an example see: vc4_drm.h
> >
> > There are a lot of UAPI that is missing documentation, but if we do not
> > add it for new UAPI then this situation never improves.
> >
> > Please use __u32, __u64 like you see in other drm UAPI files.
> >
> > PS. If you reply to this mail, then please keep the full mail as
> > usually my mails to Kevin bounces (something with STARTTLS).
> >
> >       Sam
> >
> >> ---
> >>   include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
> >>   1 file changed, 31 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> >> index a1e125d42208..e9da45ce130a 100644
> >> --- a/include/uapi/drm/via_drm.h
> >> +++ b/include/uapi/drm/via_drm.h
> >> @@ -1,4 +1,5 @@
> >>   /*
> >> + * Copyright © 2020 Kevin Brace
> >>    * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
> >>    * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
> >>    *
> >> @@ -16,10 +17,10 @@
> >>    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >>    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >>    * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> >> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> >> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> >> - * DEALINGS IN THE SOFTWARE.
> >> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> >> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> >> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> >> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >>    */
> > Do not mix license changes with other changes - and use SPDX tag if
> > possible for the updated license.
> > See other drm UAPI files for examples.
> >
> >
> >>   #ifndef _VIA_DRM_H_
> >>   #define _VIA_DRM_H_
> >> @@ -81,6 +82,11 @@ extern "C" {
> >>   #define DRM_VIA_DMA_BLIT        0x0e
> >>   #define DRM_VIA_BLIT_SYNC       0x0f
> >>
> >> +#define     DRM_VIA_GEM_CREATE      0x10
> >> +#define     DRM_VIA_GEM_MAP         0x11
> >> +#define     DRM_VIA_GEM_UNMAP       0x12
> >> +
> > Use the same alignment as the previous lines.
> >> +
> > Drop extra empty line.
> >
> >>   #define DRM_IOCTL_VIA_ALLOCMEM       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
> >>   #define DRM_IOCTL_VIA_FREEMEM        DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
> >>   #define DRM_IOCTL_VIA_AGP_INIT       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> >> @@ -97,6 +103,10 @@ extern "C" {
> >>   #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> >>   #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> >>
> >> +#define     DRM_IOCTL_VIA_GEM_CREATE        DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> >> +#define     DRM_IOCTL_VIA_GEM_MAP           DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> >> +#define     DRM_IOCTL_VIA_GEM_UNMAP         DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> >> +
> > Use same alignment as previous lines.
> >
> >>   /* Indices into buf.Setup where various bits of state are mirrored per
> >>    * context and per buffer.  These can be fired at the card as a unit,
> >>    * or in a piecewise fashion as required.
> >> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
> >>      drm_via_blitsync_t sync;
> >>   } drm_via_dmablit_t;
> >>
> >> +struct drm_via_gem_create {
> >> +    uint64_t size;
> >> +    uint32_t alignment;
> >> +    uint32_t domain;
> >> +    uint32_t handle;
> >> +    uint64_t offset;
> >> +};
> > I do not know if this is relevant, but adding a 64 bit parameter
> > (offset) that is only 32 bit aligned looks like trouble to me.
> > I hope others that knows this better can comment here.
>
> The compiler will leave a 4-byte gap between handle and offset.
> Structure allocation guarantees a minimal alignment of 8 bytes, so the
> field alignment will be correct. It's all dependend on architecture,
> platofrm, calling convention, but that's the rule of thumb.
>
> Have a look at the tool 'pahole' to inspect data-structure alignment in
> object files. You'll find plenty of gaps in compiled structure.
>
> It's still questionable to leave the gap there. Either declare it
> explicity (e.g., __u32 empty0; )  or declare the structure with
> __attribute__((__packed__)).  Personally, I'd use the former.

It's not allowed at all to use packed or leave the gap.

https://www.kernel.org/doc/html/latest/process/botching-up-ioctls.html

The 2nd prereq covers this.

Dave.

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 5843 bytes --]

Hi

Am 26.07.22 um 22:20 schrieb Dave Airlie:
> On Wed, 27 Jul 2022 at 04:18, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 26.07.22 um 19:41 schrieb Sam Ravnborg:
>>> Hi Kevin.
>>>
>>> On Mon, Jul 25, 2022 at 04:53:53PM -0700, Kevin Brace wrote:
>>>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>>
>>>> Changed the uAPI.
>>>>
>>>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>
>>> It would be great to have the new extensions to the UAPI documented
>>> using kernel-doc.
>>> As an example see: vc4_drm.h
>>>
>>> There are a lot of UAPI that is missing documentation, but if we do not
>>> add it for new UAPI then this situation never improves.
>>>
>>> Please use __u32, __u64 like you see in other drm UAPI files.
>>>
>>> PS. If you reply to this mail, then please keep the full mail as
>>> usually my mails to Kevin bounces (something with STARTTLS).
>>>
>>>        Sam
>>>
>>>> ---
>>>>    include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
>>>>    1 file changed, 31 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
>>>> index a1e125d42208..e9da45ce130a 100644
>>>> --- a/include/uapi/drm/via_drm.h
>>>> +++ b/include/uapi/drm/via_drm.h
>>>> @@ -1,4 +1,5 @@
>>>>    /*
>>>> + * Copyright © 2020 Kevin Brace
>>>>     * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
>>>>     * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
>>>>     *
>>>> @@ -16,10 +17,10 @@
>>>>     * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>>>     * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>>>     * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>>>> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>>> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>>> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>>> - * DEALINGS IN THE SOFTWARE.
>>>> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
>>>> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
>>>> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
>>>> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>>>     */
>>> Do not mix license changes with other changes - and use SPDX tag if
>>> possible for the updated license.
>>> See other drm UAPI files for examples.
>>>
>>>
>>>>    #ifndef _VIA_DRM_H_
>>>>    #define _VIA_DRM_H_
>>>> @@ -81,6 +82,11 @@ extern "C" {
>>>>    #define DRM_VIA_DMA_BLIT        0x0e
>>>>    #define DRM_VIA_BLIT_SYNC       0x0f
>>>>
>>>> +#define     DRM_VIA_GEM_CREATE      0x10
>>>> +#define     DRM_VIA_GEM_MAP         0x11
>>>> +#define     DRM_VIA_GEM_UNMAP       0x12
>>>> +
>>> Use the same alignment as the previous lines.
>>>> +
>>> Drop extra empty line.
>>>
>>>>    #define DRM_IOCTL_VIA_ALLOCMEM       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>>>>    #define DRM_IOCTL_VIA_FREEMEM        DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
>>>>    #define DRM_IOCTL_VIA_AGP_INIT       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
>>>> @@ -97,6 +103,10 @@ extern "C" {
>>>>    #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>>>>    #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
>>>>
>>>> +#define     DRM_IOCTL_VIA_GEM_CREATE        DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
>>>> +#define     DRM_IOCTL_VIA_GEM_MAP           DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
>>>> +#define     DRM_IOCTL_VIA_GEM_UNMAP         DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
>>>> +
>>> Use same alignment as previous lines.
>>>
>>>>    /* Indices into buf.Setup where various bits of state are mirrored per
>>>>     * context and per buffer.  These can be fired at the card as a unit,
>>>>     * or in a piecewise fashion as required.
>>>> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
>>>>       drm_via_blitsync_t sync;
>>>>    } drm_via_dmablit_t;
>>>>
>>>> +struct drm_via_gem_create {
>>>> +    uint64_t size;
>>>> +    uint32_t alignment;
>>>> +    uint32_t domain;
>>>> +    uint32_t handle;
>>>> +    uint64_t offset;
>>>> +};
>>> I do not know if this is relevant, but adding a 64 bit parameter
>>> (offset) that is only 32 bit aligned looks like trouble to me.
>>> I hope others that knows this better can comment here.
>>
>> The compiler will leave a 4-byte gap between handle and offset.
>> Structure allocation guarantees a minimal alignment of 8 bytes, so the
>> field alignment will be correct. It's all dependend on architecture,
>> platofrm, calling convention, but that's the rule of thumb.
>>
>> Have a look at the tool 'pahole' to inspect data-structure alignment in
>> object files. You'll find plenty of gaps in compiled structure.
>>
>> It's still questionable to leave the gap there. Either declare it
>> explicity (e.g., __u32 empty0; )  or declare the structure with
>> __attribute__((__packed__)).  Personally, I'd use the former.
> 
> It's not allowed at all to use packed or leave the gap.
> 
> https://www.kernel.org/doc/html/latest/process/botching-up-ioctls.html
> 
> The 2nd prereq covers this.

I wasn't aware of this page. Thanks.

Best regards
Thomas

> 
> Dave.

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

Hi Dave,

Thanks for pointing out the blog written by Daniel.
I was thinking that I will need to make refinements to OpenChrome DRM uAPI, so I will rework the uAPI.
I also plan to discontinue old VIA DRM uAPI by using drm_invalid_op().
No one pointed this out, but I was thinking that something like this had to be done.
I will work on these, and post the updated code soon with taking into consideration the VIA DRM DRI1 compaction work done by Sam / Thomas.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Tuesday, July 26, 2022 at 1:20 PM
> From: "Dave Airlie" <airlied@gmail.com>
> To: "Thomas Zimmermann" <tzimmermann@suse.de>
> Cc: "Sam Ravnborg" <sam@ravnborg.org>, "Kevin Brace" <kevinbrace@gmx.com>, "Kevin Brace" <kevinbrace@bracecomputerlab.com>, "dri-devel" <dri-devel@lists.freedesktop.org>
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> On Wed, 27 Jul 2022 at 04:18, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >
> > Hi
> >
> > Am 26.07.22 um 19:41 schrieb Sam Ravnborg:
> > > Hi Kevin.
> > >
> > > On Mon, Jul 25, 2022 at 04:53:53PM -0700, Kevin Brace wrote:
> > >> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > >>
> > >> Changed the uAPI.
> > >>
> > >> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > >
> > > It would be great to have the new extensions to the UAPI documented
> > > using kernel-doc.
> > > As an example see: vc4_drm.h
> > >
> > > There are a lot of UAPI that is missing documentation, but if we do not
> > > add it for new UAPI then this situation never improves.
> > >
> > > Please use __u32, __u64 like you see in other drm UAPI files.
> > >
> > > PS. If you reply to this mail, then please keep the full mail as
> > > usually my mails to Kevin bounces (something with STARTTLS).
> > >
> > >       Sam
> > >
> > >> ---
> > >>   include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
> > >>   1 file changed, 31 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> > >> index a1e125d42208..e9da45ce130a 100644
> > >> --- a/include/uapi/drm/via_drm.h
> > >> +++ b/include/uapi/drm/via_drm.h
> > >> @@ -1,4 +1,5 @@
> > >>   /*
> > >> + * Copyright © 2020 Kevin Brace
> > >>    * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
> > >>    * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
> > >>    *
> > >> @@ -16,10 +17,10 @@
> > >>    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > >>    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > >>    * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> > >> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > >> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > >> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > >> - * DEALINGS IN THE SOFTWARE.
> > >> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> > >> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> > >> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> > >> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> > >>    */
> > > Do not mix license changes with other changes - and use SPDX tag if
> > > possible for the updated license.
> > > See other drm UAPI files for examples.
> > >
> > >
> > >>   #ifndef _VIA_DRM_H_
> > >>   #define _VIA_DRM_H_
> > >> @@ -81,6 +82,11 @@ extern "C" {
> > >>   #define DRM_VIA_DMA_BLIT        0x0e
> > >>   #define DRM_VIA_BLIT_SYNC       0x0f
> > >>
> > >> +#define     DRM_VIA_GEM_CREATE      0x10
> > >> +#define     DRM_VIA_GEM_MAP         0x11
> > >> +#define     DRM_VIA_GEM_UNMAP       0x12
> > >> +
> > > Use the same alignment as the previous lines.
> > >> +
> > > Drop extra empty line.
> > >
> > >>   #define DRM_IOCTL_VIA_ALLOCMEM       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
> > >>   #define DRM_IOCTL_VIA_FREEMEM        DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
> > >>   #define DRM_IOCTL_VIA_AGP_INIT       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> > >> @@ -97,6 +103,10 @@ extern "C" {
> > >>   #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> > >>   #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> > >>
> > >> +#define     DRM_IOCTL_VIA_GEM_CREATE        DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> > >> +#define     DRM_IOCTL_VIA_GEM_MAP           DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> > >> +#define     DRM_IOCTL_VIA_GEM_UNMAP         DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> > >> +
> > > Use same alignment as previous lines.
> > >
> > >>   /* Indices into buf.Setup where various bits of state are mirrored per
> > >>    * context and per buffer.  These can be fired at the card as a unit,
> > >>    * or in a piecewise fashion as required.
> > >> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
> > >>      drm_via_blitsync_t sync;
> > >>   } drm_via_dmablit_t;
> > >>
> > >> +struct drm_via_gem_create {
> > >> +    uint64_t size;
> > >> +    uint32_t alignment;
> > >> +    uint32_t domain;
> > >> +    uint32_t handle;
> > >> +    uint64_t offset;
> > >> +};
> > > I do not know if this is relevant, but adding a 64 bit parameter
> > > (offset) that is only 32 bit aligned looks like trouble to me.
> > > I hope others that knows this better can comment here.
> >
> > The compiler will leave a 4-byte gap between handle and offset.
> > Structure allocation guarantees a minimal alignment of 8 bytes, so the
> > field alignment will be correct. It's all dependend on architecture,
> > platofrm, calling convention, but that's the rule of thumb.
> >
> > Have a look at the tool 'pahole' to inspect data-structure alignment in
> > object files. You'll find plenty of gaps in compiled structure.
> >
> > It's still questionable to leave the gap there. Either declare it
> > explicity (e.g., __u32 empty0; )  or declare the structure with
> > __attribute__((__packed__)).  Personally, I'd use the former.
> 
> It's not allowed at all to use packed or leave the gap.
> 
> https://www.kernel.org/doc/html/latest/process/botching-up-ioctls.html
> 
> The 2nd prereq covers this.
> 
> Dave.
>

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

Hi Sam,

I will study vc4_drm.h and update newer portions (i.e., the section I am actively adding to via_drm.h) with new comments that can be converted to kernel-doc automatically.
I also plan to rework the uAPI somewhat.

- Remove DRM_VIA_GEM_UNMAP
- Revive a uAPI that can pass a PCI Device ID back to userspace.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Tuesday, July 26, 2022 at 10:41 AM
> From: "Sam Ravnborg" <sam@ravnborg.org>
> To: "Kevin Brace" <kevinbrace@gmx.com>
> Cc: dri-devel@lists.freedesktop.org, "Kevin Brace" <kevinbrace@bracecomputerlab.com>
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> Hi Kevin.
> 
> On Mon, Jul 25, 2022 at 04:53:53PM -0700, Kevin Brace wrote:
> > From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > 
> > Changed the uAPI.
> > 
> > Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> It would be great to have the new extensions to the UAPI documented
> using kernel-doc.
> As an example see: vc4_drm.h
> 
> There are a lot of UAPI that is missing documentation, but if we do not
> add it for new UAPI then this situation never improves.
> 
> Please use __u32, __u64 like you see in other drm UAPI files.
> 
> PS. If you reply to this mail, then please keep the full mail as
> usually my mails to Kevin bounces (something with STARTTLS).
> 
> 	Sam
> 
> > ---
> >  include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
> >  1 file changed, 31 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> > index a1e125d42208..e9da45ce130a 100644
> > --- a/include/uapi/drm/via_drm.h
> > +++ b/include/uapi/drm/via_drm.h
> > @@ -1,4 +1,5 @@
> >  /*
> > + * Copyright © 2020 Kevin Brace
> >   * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
> >   * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
> >   *
> > @@ -16,10 +17,10 @@
> >   * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >   * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >   * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> > - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > - * DEALINGS IN THE SOFTWARE.
> > + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> > + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> > + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> > + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >   */
> Do not mix license changes with other changes - and use SPDX tag if
> possible for the updated license.
> See other drm UAPI files for examples.
> 
> 
> >  #ifndef _VIA_DRM_H_
> >  #define _VIA_DRM_H_
> > @@ -81,6 +82,11 @@ extern "C" {
> >  #define DRM_VIA_DMA_BLIT        0x0e
> >  #define DRM_VIA_BLIT_SYNC       0x0f
> > 
> > +#define	DRM_VIA_GEM_CREATE	0x10
> > +#define	DRM_VIA_GEM_MAP		0x11
> > +#define	DRM_VIA_GEM_UNMAP	0x12
> > +
> Use the same alignment as the previous lines.
> > +
> Drop extra empty line.
> 
> >  #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
> >  #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
> >  #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> > @@ -97,6 +103,10 @@ extern "C" {
> >  #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> >  #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> > 
> > +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> > +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> > +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> > +
> Use same alignment as previous lines.
> 
> >  /* Indices into buf.Setup where various bits of state are mirrored per
> >   * context and per buffer.  These can be fired at the card as a unit,
> >   * or in a piecewise fashion as required.
> > @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
> >  	drm_via_blitsync_t sync;
> >  } drm_via_dmablit_t;
> > 
> > +struct drm_via_gem_create {
> > +	uint64_t size;
> > +	uint32_t alignment;
> > +	uint32_t domain;
> > +	uint32_t handle;
> > +	uint64_t offset;
> > +};
> I do not know if this is relevant, but adding a 64 bit parameter
> (offset) that is only 32 bit aligned looks like trouble to me.
> I hope others that knows this better can comment here.
> 
> > +
> > +struct drm_via_gem_map {
> > +	uint32_t handle;
> > +	uint64_t map_offset;
> > +};
> Same here with the alignment.
> 
> If drm_via_gem_create.offset and drm_via_gem_map.map_offset is the same
> the field should have the same name - to make the API easier to use.
> 
> 
> > +
> > +struct drm_via_gem_unmap {
> > +	uint32_t handle;
> > +};
> > +
> >  #if defined(__cplusplus)
> >  }
> >  #endif
> > --
> > 2.35.1
>

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3996 bytes --]

Hi

Am 26.07.22 um 01:53 schrieb Kevin Brace:
> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> Changed the uAPI.
> 
> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> ---
>   include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
>   1 file changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> index a1e125d42208..e9da45ce130a 100644
> --- a/include/uapi/drm/via_drm.h
> +++ b/include/uapi/drm/via_drm.h
> @@ -1,4 +1,5 @@
>   /*
> + * Copyright © 2020 Kevin Brace
>    * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
>    * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
>    *
> @@ -16,10 +17,10 @@
>    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>    * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> - * DEALINGS IN THE SOFTWARE.
> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>    */
>   #ifndef _VIA_DRM_H_
>   #define _VIA_DRM_H_
> @@ -81,6 +82,11 @@ extern "C" {
>   #define DRM_VIA_DMA_BLIT        0x0e
>   #define DRM_VIA_BLIT_SYNC       0x0f
> 
> +#define	DRM_VIA_GEM_CREATE	0x10
> +#define	DRM_VIA_GEM_MAP		0x11
> +#define	DRM_VIA_GEM_UNMAP	0x12

This looks a lot like ioctl ops for using accelerated HW buffers. But 
the patch is near the end of the series and you said in the series' 
cover letter that there's no acceleration. I suspect that these 
constants are currently unused?  If so, please drop the patch from the 
series. If can be merged later when something really depends on it.

Best regards
Thomas

> +
> +
>   #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>   #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
>   #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> @@ -97,6 +103,10 @@ extern "C" {
>   #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>   #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> 
> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> +
>   /* Indices into buf.Setup where various bits of state are mirrored per
>    * context and per buffer.  These can be fired at the card as a unit,
>    * or in a piecewise fashion as required.
> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
>   	drm_via_blitsync_t sync;
>   } drm_via_dmablit_t;
> 
> +struct drm_via_gem_create {
> +	uint64_t size;
> +	uint32_t alignment;
> +	uint32_t domain;
> +	uint32_t handle;
> +	uint64_t offset;
> +};
> +
> +struct drm_via_gem_map {
> +	uint32_t handle;
> +	uint64_t map_offset;
> +};
> +
> +struct drm_via_gem_unmap {
> +	uint32_t handle;
> +};
> +
>   #if defined(__cplusplus)
>   }
>   #endif
> --
> 2.35.1
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

Hi Thomas,

I cannot drop the older DRI1 based uAPI calls.
This is because include/uapi/drm/via_drm.h needs to retain backward compatibility with the existing OpenChrome DDX's XvMC library (it gets compiled when OpenChrome DDX is built) and likely with the existing DDX Xv code as well.
If I remove the DRI1 based uAPI calls, the XvMC library will not get compiled (compile error will occur since the XvMC library assumes the presence of DRI1 based uAPI), and I assume the same for the DDX Xv code (I cannot even reach here since the XvMC library is compiled first).
Although the v3 patch does not contain it, v4 patch will utilize drm_invalid_op() for the discontinued (not deprecated since OpenChrome DRM does not support the older DRI1 based uAPI at all) DRI1 based uAPI.

https://cgit.freedesktop.org/openchrome/drm-openchrome/commit/?h=drm-next-5.20&id=16b3d68f95c9ccd15b7a3310e5d752fabbc76518

drm_invalid_op() is related to drm_ioctl.c, and is meant for legacy DRMs like Radeon, i915, etc.
Since OpenChrome DRM is not a clean sheet design (related to VIA DRM to some extent), I will use this function for properly handling discontinued legacy uAPI calls.
I hope this explanation / reasoning is okay with you.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com

> Sent: Tuesday, July 26, 2022 at 11:24 AM
> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> Hi
> 
> Am 26.07.22 um 01:53 schrieb Kevin Brace:
> > From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > 
> > Changed the uAPI.
> > 
> > Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > ---
> >   include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
> >   1 file changed, 31 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> > index a1e125d42208..e9da45ce130a 100644
> > --- a/include/uapi/drm/via_drm.h
> > +++ b/include/uapi/drm/via_drm.h
> > @@ -1,4 +1,5 @@
> >   /*
> > + * Copyright © 2020 Kevin Brace
> >    * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
> >    * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
> >    *
> > @@ -16,10 +17,10 @@
> >    * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >    * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >    * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> > - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > - * DEALINGS IN THE SOFTWARE.
> > + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> > + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> > + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> > + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >    */
> >   #ifndef _VIA_DRM_H_
> >   #define _VIA_DRM_H_
> > @@ -81,6 +82,11 @@ extern "C" {
> >   #define DRM_VIA_DMA_BLIT        0x0e
> >   #define DRM_VIA_BLIT_SYNC       0x0f
> > 
> > +#define	DRM_VIA_GEM_CREATE	0x10
> > +#define	DRM_VIA_GEM_MAP		0x11
> > +#define	DRM_VIA_GEM_UNMAP	0x12
> 
> This looks a lot like ioctl ops for using accelerated HW buffers. But 
> the patch is near the end of the series and you said in the series' 
> cover letter that there's no acceleration. I suspect that these 
> constants are currently unused?  If so, please drop the patch from the 
> series. If can be merged later when something really depends on it.
> 
> Best regards
> Thomas
> 
> > +
> > +
> >   #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
> >   #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
> >   #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> > @@ -97,6 +103,10 @@ extern "C" {
> >   #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> >   #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> > 
> > +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> > +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> > +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> > +
> >   /* Indices into buf.Setup where various bits of state are mirrored per
> >    * context and per buffer.  These can be fired at the card as a unit,
> >    * or in a piecewise fashion as required.
> > @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
> >   	drm_via_blitsync_t sync;
> >   } drm_via_dmablit_t;
> > 
> > +struct drm_via_gem_create {
> > +	uint64_t size;
> > +	uint32_t alignment;
> > +	uint32_t domain;
> > +	uint32_t handle;
> > +	uint64_t offset;
> > +};
> > +
> > +struct drm_via_gem_map {
> > +	uint32_t handle;
> > +	uint64_t map_offset;
> > +};
> > +
> > +struct drm_via_gem_unmap {
> > +	uint32_t handle;
> > +};
> > +
> >   #if defined(__cplusplus)
> >   }
> >   #endif
> > --
> > 2.35.1
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
>

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Sam Ravnborg]

Hi Kevin,

> I cannot drop the older DRI1 based uAPI calls.
> This is because include/uapi/drm/via_drm.h needs to retain backward
> compatibility with the existing OpenChrome DDX's XvMC library
> (it gets compiled when OpenChrome DDX is built) and likely with the
> existing DDX Xv code as well.
> If I remove the DRI1 based uAPI calls, the XvMC library will not get
> compiled (compile error will occur since the XvMC library assumes the
> presence of DRI1 based uAPI), and I assume the same for the DDX Xv code
> (I cannot even reach here since the XvMC library is compiled first).

If you just keep the relevant definitions in drm_via.h then the compile
issues should be OK - and then there is no need to implement anything in
the driver. Or did I not understand the problem you are trying to solve?

	Sam

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

Hi Sam,

OpenChrome DDX carries lots of legacy code.

https://cgit.freedesktop.org/openchrome/xf86-video-openchrome/tree/src/via_drm.h?h=main&id=dc661c59257e855cd9b29c14b91a8ee2d9b86ccb

There is a requirement to use the same via_drm.h with both DDX and DRM.
Hence, I need to keep a lot of the legacy DRI1 definitions inside via_drm.h.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Monday, August 01, 2022 at 7:40 AM
> From: "Sam Ravnborg" <sam@ravnborg.org>
> To: "Kevin Brace" <kevinbrace@gmx.com>
> Cc: "Thomas Zimmermann" <tzimmermann@suse.de>, dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> Hi Kevin,
>
> > I cannot drop the older DRI1 based uAPI calls.
> > This is because include/uapi/drm/via_drm.h needs to retain backward
> > compatibility with the existing OpenChrome DDX's XvMC library
> > (it gets compiled when OpenChrome DDX is built) and likely with the
> > existing DDX Xv code as well.
> > If I remove the DRI1 based uAPI calls, the XvMC library will not get
> > compiled (compile error will occur since the XvMC library assumes the
> > presence of DRI1 based uAPI), and I assume the same for the DDX Xv code
> > (I cannot even reach here since the XvMC library is compiled first).
>
> If you just keep the relevant definitions in drm_via.h then the compile
> issues should be OK - and then there is no need to implement anything in
> the driver. Or did I not understand the problem you are trying to solve?
>
> 	Sam
>

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Sam Ravnborg]

Hi Kevin,
> 
> OpenChrome DDX carries lots of legacy code.
> 
> https://cgit.freedesktop.org/openchrome/xf86-video-openchrome/tree/src/via_drm.h?h=main&id=dc661c59257e855cd9b29c14b91a8ee2d9b86ccb
> 
> There is a requirement to use the same via_drm.h with both DDX and DRM.
> Hence, I need to keep a lot of the legacy DRI1 definitions inside via_drm.h.

This part is fully understood. Also on top of this the via DRI1 driver
uses this. I am not asking to have anything deleted from the existing
uapi via_drm.h file.


My feedback is that the following code should be dropped from the
openchrome driver:

+	DRM_IOCTL_DEF_DRV(VIA_ALLOCMEM, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_FREEMEM, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_AGP_INIT, drm_invalid_op, DRM_AUTH | DRM_MASTER),
+	DRM_IOCTL_DEF_DRV(VIA_FB_INIT, drm_invalid_op, DRM_AUTH | DRM_MASTER),
+	DRM_IOCTL_DEF_DRV(VIA_MAP_INIT, drm_invalid_op, DRM_AUTH | DRM_MASTER),
+	DRM_IOCTL_DEF_DRV(VIA_DEC_FUTEX, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_DMA_INIT, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_CMDBUFFER, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_FLUSH, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_PCICMD, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_CMDBUF_SIZE, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_WAIT_IRQ, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_DMA_BLIT, drm_invalid_op, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(VIA_BLIT_SYNC, drm_invalid_op, DRM_AUTH),

(Copied from openchrome-drm - I recall you did not post this code yet).

The new openchrome driver should not care at all about the old UAPI,
so just drop the above.

The comment above is based on the understanding that when we have a kms
compliant driver the user space is generic and we do not expect or need
any via specifics in user space.

In other words - x86-video-openchrome should - according to my
understanding - not be needed. And we can have a fully operational
wayland (and maybe X) userspace using the generic UAPI. This is where
Thomas Zimmermann's comment about dumb buffers are relevant.

Do I miss something obvious here?

	Sam

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

Hi Sam,

Regarding DRI1 era uAPI, I believe I am handling the matter similar to how Radeon DRM header (include/uapi/drm/radeon_drm.h) handled the matter.
The header still contains old DRI1 uAPI calls, and it then adds new KMS generation uAPI calls.
At this point, using drm_invalid_op() for OpenChrome DRM is the least intrusive option, and that's the way I will like to keep it.
It is just that I did not know its existence, so it was not in the code.
The older OpenChrome DDX releases might assume uAPI backward compatibility, but the use of drm_invalid_op() should gracefully tell the DDX that the legacy DRI1 VIA DRM uAPI is no longer supported.
    Personally, I do not anticipate Wayland use with OpenChrome.
It will end up living out its life as X.Org X Server only solution considering its hardware age.
Although it is somewhat hard, I use Gentoo Linux as development platform for OpenChrome, and 32-bit x86 ISA and X.Org X Server are still fully supported.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Tuesday, August 02, 2022 at 4:09 AM
> From: "Sam Ravnborg" <sam@ravnborg.org>
> To: "Kevin Brace" <kevinbrace@gmx.com>
> Cc: "Thomas Zimmermann" <tzimmermann@suse.de>, dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> Hi Kevin,
> >
> > OpenChrome DDX carries lots of legacy code.
> >
> > https://cgit.freedesktop.org/openchrome/xf86-video-openchrome/tree/src/via_drm.h?h=main&id=dc661c59257e855cd9b29c14b91a8ee2d9b86ccb
> >
> > There is a requirement to use the same via_drm.h with both DDX and DRM.
> > Hence, I need to keep a lot of the legacy DRI1 definitions inside via_drm.h.
>
> This part is fully understood. Also on top of this the via DRI1 driver
> uses this. I am not asking to have anything deleted from the existing
> uapi via_drm.h file.
>
>
> My feedback is that the following code should be dropped from the
> openchrome driver:
>
> +	DRM_IOCTL_DEF_DRV(VIA_ALLOCMEM, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_FREEMEM, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_AGP_INIT, drm_invalid_op, DRM_AUTH | DRM_MASTER),
> +	DRM_IOCTL_DEF_DRV(VIA_FB_INIT, drm_invalid_op, DRM_AUTH | DRM_MASTER),
> +	DRM_IOCTL_DEF_DRV(VIA_MAP_INIT, drm_invalid_op, DRM_AUTH | DRM_MASTER),
> +	DRM_IOCTL_DEF_DRV(VIA_DEC_FUTEX, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_DMA_INIT, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_CMDBUFFER, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_FLUSH, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_PCICMD, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_CMDBUF_SIZE, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_WAIT_IRQ, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_DMA_BLIT, drm_invalid_op, DRM_AUTH),
> +	DRM_IOCTL_DEF_DRV(VIA_BLIT_SYNC, drm_invalid_op, DRM_AUTH),
>
> (Copied from openchrome-drm - I recall you did not post this code yet).
>
> The new openchrome driver should not care at all about the old UAPI,
> so just drop the above.
>
> The comment above is based on the understanding that when we have a kms
> compliant driver the user space is generic and we do not expect or need
> any via specifics in user space.
>
> In other words - x86-video-openchrome should - according to my
> understanding - not be needed. And we can have a fully operational
> wayland (and maybe X) userspace using the generic UAPI. This is where
> Thomas Zimmermann's comment about dumb buffers are relevant.
>
> Do I miss something obvious here?
>
> 	Sam
>

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 6725 bytes --]

Hi Kevin

Am 31.07.22 um 00:48 schrieb Kevin Brace:
> Hi Thomas,
> 
> I cannot drop the older DRI1 based uAPI calls.
> This is because include/uapi/drm/via_drm.h needs to retain backward compatibility with the existing OpenChrome DDX's XvMC library (it gets compiled when OpenChrome DDX is built) and likely with the existing DDX Xv code as well.
> If I remove the DRI1 based uAPI calls, the XvMC library will not get compiled (compile error will occur since the XvMC library assumes the presence of DRI1 based uAPI), and I assume the same for the DDX Xv code (I cannot even reach here since the XvMC library is compiled first).
> Although the v3 patch does not contain it, v4 patch will utilize drm_invalid_op() for the discontinued (not deprecated since OpenChrome DRM does not support the older DRI1 based uAPI at all) DRI1 based uAPI.
> 
> https://cgit.freedesktop.org/openchrome/drm-openchrome/commit/?h=drm-next-5.20&id=16b3d68f95c9ccd15b7a3310e5d752fabbc76518
> 
> drm_invalid_op() is related to drm_ioctl.c, and is meant for legacy DRMs like Radeon, i915, etc.
> Since OpenChrome DRM is not a clean sheet design (related to VIA DRM to some extent), I will use this function for properly handling discontinued legacy uAPI calls.
> I hope this explanation / reasoning is okay with you.

I'm not sure I understand your reply ormaybe I'm just missing something 
here.

I'm not asking you to remove the existing DRI1 uapi. I'm just asking to 
not add the 6 new _GEM_ defines and 3 new _gem_ structures now.  You 
mentioned that the driver does not yet support acceleration of any kind. 
So there should be no need to extend to uapi now.  You can still do this 
when you add acceleration to the driver.

Until then, the Xorg modesetting driver or any Compositor can use the 
generic dumb-buffer ioctls that create buffers with no acceleration.

Best regards
Thomas

> 
> Regards,
> 
> Kevin Brace
> Brace Computer Laboratory blog
> https://bracecomputerlab.com
> 
>> Sent: Tuesday, July 26, 2022 at 11:24 AM
>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
>> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
>> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
>> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>>
>> Hi
>>
>> Am 26.07.22 um 01:53 schrieb Kevin Brace:
>>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>
>>> Changed the uAPI.
>>>
>>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>> ---
>>>    include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
>>>    1 file changed, 31 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
>>> index a1e125d42208..e9da45ce130a 100644
>>> --- a/include/uapi/drm/via_drm.h
>>> +++ b/include/uapi/drm/via_drm.h
>>> @@ -1,4 +1,5 @@
>>>    /*
>>> + * Copyright © 2020 Kevin Brace
>>>     * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
>>>     * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
>>>     *
>>> @@ -16,10 +17,10 @@
>>>     * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>>     * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>>     * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>>> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>> - * DEALINGS IN THE SOFTWARE.
>>> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
>>> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
>>> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
>>> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>>     */
>>>    #ifndef _VIA_DRM_H_
>>>    #define _VIA_DRM_H_
>>> @@ -81,6 +82,11 @@ extern "C" {
>>>    #define DRM_VIA_DMA_BLIT        0x0e
>>>    #define DRM_VIA_BLIT_SYNC       0x0f
>>>
>>> +#define	DRM_VIA_GEM_CREATE	0x10
>>> +#define	DRM_VIA_GEM_MAP		0x11
>>> +#define	DRM_VIA_GEM_UNMAP	0x12
>>
>> This looks a lot like ioctl ops for using accelerated HW buffers. But
>> the patch is near the end of the series and you said in the series'
>> cover letter that there's no acceleration. I suspect that these
>> constants are currently unused?  If so, please drop the patch from the
>> series. If can be merged later when something really depends on it.
>>
>> Best regards
>> Thomas
>>
>>> +
>>> +
>>>    #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>>>    #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
>>>    #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
>>> @@ -97,6 +103,10 @@ extern "C" {
>>>    #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>>>    #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
>>>
>>> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
>>> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
>>> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
>>> +
>>>    /* Indices into buf.Setup where various bits of state are mirrored per
>>>     * context and per buffer.  These can be fired at the card as a unit,
>>>     * or in a piecewise fashion as required.
>>> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
>>>    	drm_via_blitsync_t sync;
>>>    } drm_via_dmablit_t;
>>>
>>> +struct drm_via_gem_create {
>>> +	uint64_t size;
>>> +	uint32_t alignment;
>>> +	uint32_t domain;
>>> +	uint32_t handle;
>>> +	uint64_t offset;
>>> +};
>>> +
>>> +struct drm_via_gem_map {
>>> +	uint32_t handle;
>>> +	uint64_t map_offset;
>>> +};
>>> +
>>> +struct drm_via_gem_unmap {
>>> +	uint32_t handle;
>>> +};
>>> +
>>>    #if defined(__cplusplus)
>>>    }
>>>    #endif
>>> --
>>> 2.35.1
>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

Hi Thomas,

I hope I am comprehending this right.
Yes, I am adding 3 new uAPI calls, not 6 of them.
Correspondingly, there are 3 new structs added.
While I may drop one (unmap uAPI), I personally do not wish to drop the other two at this point.
Instead, I may need to add setparam and / or getparam uAPI to pass PCI Device ID back to the userspace.
This is mainly needed by Mesa, although there is no code for Mesa at this point.
I fear dropping the remaining two will require substantial redesign, and I will like to avoid this since the code is already working.
It is my plan to proceed to adding acceleration after the code is added to the mainline kernel tree, so I will like to do it the way it is set up now.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Monday, August 01, 2022 at 11:49 AM
> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> To: "Kevin Brace" <kevinbrace@gmx.com>
> Cc: dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> Hi Kevin
> 
> Am 31.07.22 um 00:48 schrieb Kevin Brace:
> > Hi Thomas,
> > 
> > I cannot drop the older DRI1 based uAPI calls.
> > This is because include/uapi/drm/via_drm.h needs to retain backward compatibility with the existing OpenChrome DDX's XvMC library (it gets compiled when OpenChrome DDX is built) and likely with the existing DDX Xv code as well.
> > If I remove the DRI1 based uAPI calls, the XvMC library will not get compiled (compile error will occur since the XvMC library assumes the presence of DRI1 based uAPI), and I assume the same for the DDX Xv code (I cannot even reach here since the XvMC library is compiled first).
> > Although the v3 patch does not contain it, v4 patch will utilize drm_invalid_op() for the discontinued (not deprecated since OpenChrome DRM does not support the older DRI1 based uAPI at all) DRI1 based uAPI.
> > 
> > https://cgit.freedesktop.org/openchrome/drm-openchrome/commit/?h=drm-next-5.20&id=16b3d68f95c9ccd15b7a3310e5d752fabbc76518
> > 
> > drm_invalid_op() is related to drm_ioctl.c, and is meant for legacy DRMs like Radeon, i915, etc.
> > Since OpenChrome DRM is not a clean sheet design (related to VIA DRM to some extent), I will use this function for properly handling discontinued legacy uAPI calls.
> > I hope this explanation / reasoning is okay with you.
> 
> I'm not sure I understand your reply ormaybe I'm just missing something 
> here.
> 
> I'm not asking you to remove the existing DRI1 uapi. I'm just asking to 
> not add the 6 new _GEM_ defines and 3 new _gem_ structures now.  You 
> mentioned that the driver does not yet support acceleration of any kind. 
> So there should be no need to extend to uapi now.  You can still do this 
> when you add acceleration to the driver.
> 
> Until then, the Xorg modesetting driver or any Compositor can use the 
> generic dumb-buffer ioctls that create buffers with no acceleration.
> 
> Best regards
> Thomas
> 
> > 
> > Regards,
> > 
> > Kevin Brace
> > Brace Computer Laboratory blog
> > https://bracecomputerlab.com
> > 
> >> Sent: Tuesday, July 26, 2022 at 11:24 AM
> >> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> >> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
> >> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
> >> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
> >>
> >> Hi
> >>
> >> Am 26.07.22 um 01:53 schrieb Kevin Brace:
> >>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> >>>
> >>> Changed the uAPI.
> >>>
> >>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> >>> ---
> >>>    include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
> >>>    1 file changed, 31 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> >>> index a1e125d42208..e9da45ce130a 100644
> >>> --- a/include/uapi/drm/via_drm.h
> >>> +++ b/include/uapi/drm/via_drm.h
> >>> @@ -1,4 +1,5 @@
> >>>    /*
> >>> + * Copyright © 2020 Kevin Brace
> >>>     * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
> >>>     * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
> >>>     *
> >>> @@ -16,10 +17,10 @@
> >>>     * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >>>     * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >>>     * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> >>> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> >>> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >>> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> >>> - * DEALINGS IN THE SOFTWARE.
> >>> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> >>> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> >>> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> >>> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >>>     */
> >>>    #ifndef _VIA_DRM_H_
> >>>    #define _VIA_DRM_H_
> >>> @@ -81,6 +82,11 @@ extern "C" {
> >>>    #define DRM_VIA_DMA_BLIT        0x0e
> >>>    #define DRM_VIA_BLIT_SYNC       0x0f
> >>>
> >>> +#define	DRM_VIA_GEM_CREATE	0x10
> >>> +#define	DRM_VIA_GEM_MAP		0x11
> >>> +#define	DRM_VIA_GEM_UNMAP	0x12
> >>
> >> This looks a lot like ioctl ops for using accelerated HW buffers. But
> >> the patch is near the end of the series and you said in the series'
> >> cover letter that there's no acceleration. I suspect that these
> >> constants are currently unused?  If so, please drop the patch from the
> >> series. If can be merged later when something really depends on it.
> >>
> >> Best regards
> >> Thomas
> >>
> >>> +
> >>> +
> >>>    #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
> >>>    #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
> >>>    #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> >>> @@ -97,6 +103,10 @@ extern "C" {
> >>>    #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> >>>    #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> >>>
> >>> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> >>> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> >>> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> >>> +
> >>>    /* Indices into buf.Setup where various bits of state are mirrored per
> >>>     * context and per buffer.  These can be fired at the card as a unit,
> >>>     * or in a piecewise fashion as required.
> >>> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
> >>>    	drm_via_blitsync_t sync;
> >>>    } drm_via_dmablit_t;
> >>>
> >>> +struct drm_via_gem_create {
> >>> +	uint64_t size;
> >>> +	uint32_t alignment;
> >>> +	uint32_t domain;
> >>> +	uint32_t handle;
> >>> +	uint64_t offset;
> >>> +};
> >>> +
> >>> +struct drm_via_gem_map {
> >>> +	uint32_t handle;
> >>> +	uint64_t map_offset;
> >>> +};
> >>> +
> >>> +struct drm_via_gem_unmap {
> >>> +	uint32_t handle;
> >>> +};
> >>> +
> >>>    #if defined(__cplusplus)
> >>>    }
> >>>    #endif
> >>> --
> >>> 2.35.1
> >>>
> >>
> >> -- 
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> (HRB 36809, AG Nürnberg)
> >> Geschäftsführer: Ivo Totev
> >>
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
>

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 9488 bytes --]

Hi

Am 02.08.22 um 05:45 schrieb Kevin Brace:
> Hi Thomas,
> 
> I hope I am comprehending this right.
> Yes, I am adding 3 new uAPI calls, not 6 of them.
> Correspondingly, there are 3 new structs added.

That's understood.

> While I may drop one (unmap uAPI), I personally do not wish to drop the other two at this point.
> Instead, I may need to add setparam and / or getparam uAPI to pass PCI Device ID back to the userspace.
> This is mainly needed by Mesa, although there is no code for Mesa at this point.

Exactly my point! There's no userspace for it. That's why Sam and me are 
asking you to remove all kinds if uapi changes or ioctls from the 
patchset until Mesa (or some other component) requires it.

> I fear dropping the remaining two will require substantial redesign, and I will like to avoid this since the code is already working.

No, it won't require a redesign. You'll have to remove the changes to 
the uapi header and any new ioctls that are in the patchset. Userspace 
programs; such as X11's modesetting driver, Weston or Gnome; will use 
the kernel's dumb-buffer ioctls to create unaccelerated buffers.  You 
won't need any via-specific code in userspace. It's all there already 
and fully driver independent. Mesa will do software rendering.  For the 
kernel's dumb buffers, please see [1].

> It is my plan to proceed to adding acceleration after the code is added to the mainline kernel tree, so I will like to do it the way it is set up now.

You can still send the current uapi changes when you add 3d acceleration 
to the kernel and Mesa.  But once these interfaces have been added to 
the kernel, they are nearly impossible to change or remove. That's why 
we don't want to do this now.

Best regards
Thomas

[1] 
https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#dumb-buffer-objects

> 
> Regards,
> 
> Kevin Brace
> Brace Computer Laboratory blog
> https://bracecomputerlab.com
> 
> 
>> Sent: Monday, August 01, 2022 at 11:49 AM
>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
>> To: "Kevin Brace" <kevinbrace@gmx.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>>
>> Hi Kevin
>>
>> Am 31.07.22 um 00:48 schrieb Kevin Brace:
>>> Hi Thomas,
>>>
>>> I cannot drop the older DRI1 based uAPI calls.
>>> This is because include/uapi/drm/via_drm.h needs to retain backward compatibility with the existing OpenChrome DDX's XvMC library (it gets compiled when OpenChrome DDX is built) and likely with the existing DDX Xv code as well.
>>> If I remove the DRI1 based uAPI calls, the XvMC library will not get compiled (compile error will occur since the XvMC library assumes the presence of DRI1 based uAPI), and I assume the same for the DDX Xv code (I cannot even reach here since the XvMC library is compiled first).
>>> Although the v3 patch does not contain it, v4 patch will utilize drm_invalid_op() for the discontinued (not deprecated since OpenChrome DRM does not support the older DRI1 based uAPI at all) DRI1 based uAPI.
>>>
>>> https://cgit.freedesktop.org/openchrome/drm-openchrome/commit/?h=drm-next-5.20&id=16b3d68f95c9ccd15b7a3310e5d752fabbc76518
>>>
>>> drm_invalid_op() is related to drm_ioctl.c, and is meant for legacy DRMs like Radeon, i915, etc.
>>> Since OpenChrome DRM is not a clean sheet design (related to VIA DRM to some extent), I will use this function for properly handling discontinued legacy uAPI calls.
>>> I hope this explanation / reasoning is okay with you.
>>
>> I'm not sure I understand your reply ormaybe I'm just missing something
>> here.
>>
>> I'm not asking you to remove the existing DRI1 uapi. I'm just asking to
>> not add the 6 new _GEM_ defines and 3 new _gem_ structures now.  You
>> mentioned that the driver does not yet support acceleration of any kind.
>> So there should be no need to extend to uapi now.  You can still do this
>> when you add acceleration to the driver.
>>
>> Until then, the Xorg modesetting driver or any Compositor can use the
>> generic dumb-buffer ioctls that create buffers with no acceleration.
>>
>> Best regards
>> Thomas
>>
>>>
>>> Regards,
>>>
>>> Kevin Brace
>>> Brace Computer Laboratory blog
>>> https://bracecomputerlab.com
>>>
>>>> Sent: Tuesday, July 26, 2022 at 11:24 AM
>>>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
>>>> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
>>>> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
>>>> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>>>>
>>>> Hi
>>>>
>>>> Am 26.07.22 um 01:53 schrieb Kevin Brace:
>>>>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>>>
>>>>> Changed the uAPI.
>>>>>
>>>>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>>> ---
>>>>>     include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
>>>>>     1 file changed, 31 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
>>>>> index a1e125d42208..e9da45ce130a 100644
>>>>> --- a/include/uapi/drm/via_drm.h
>>>>> +++ b/include/uapi/drm/via_drm.h
>>>>> @@ -1,4 +1,5 @@
>>>>>     /*
>>>>> + * Copyright © 2020 Kevin Brace
>>>>>      * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
>>>>>      * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
>>>>>      *
>>>>> @@ -16,10 +17,10 @@
>>>>>      * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>>>>      * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>>>>      * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>>>>> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>>>> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>>>> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>>>> - * DEALINGS IN THE SOFTWARE.
>>>>> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
>>>>> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
>>>>> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
>>>>> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>>>>      */
>>>>>     #ifndef _VIA_DRM_H_
>>>>>     #define _VIA_DRM_H_
>>>>> @@ -81,6 +82,11 @@ extern "C" {
>>>>>     #define DRM_VIA_DMA_BLIT        0x0e
>>>>>     #define DRM_VIA_BLIT_SYNC       0x0f
>>>>>
>>>>> +#define	DRM_VIA_GEM_CREATE	0x10
>>>>> +#define	DRM_VIA_GEM_MAP		0x11
>>>>> +#define	DRM_VIA_GEM_UNMAP	0x12
>>>>
>>>> This looks a lot like ioctl ops for using accelerated HW buffers. But
>>>> the patch is near the end of the series and you said in the series'
>>>> cover letter that there's no acceleration. I suspect that these
>>>> constants are currently unused?  If so, please drop the patch from the
>>>> series. If can be merged later when something really depends on it.
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>> +
>>>>> +
>>>>>     #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>>>>>     #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
>>>>>     #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
>>>>> @@ -97,6 +103,10 @@ extern "C" {
>>>>>     #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>>>>>     #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
>>>>>
>>>>> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
>>>>> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
>>>>> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
>>>>> +
>>>>>     /* Indices into buf.Setup where various bits of state are mirrored per
>>>>>      * context and per buffer.  These can be fired at the card as a unit,
>>>>>      * or in a piecewise fashion as required.
>>>>> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
>>>>>     	drm_via_blitsync_t sync;
>>>>>     } drm_via_dmablit_t;
>>>>>
>>>>> +struct drm_via_gem_create {
>>>>> +	uint64_t size;
>>>>> +	uint32_t alignment;
>>>>> +	uint32_t domain;
>>>>> +	uint32_t handle;
>>>>> +	uint64_t offset;
>>>>> +};
>>>>> +
>>>>> +struct drm_via_gem_map {
>>>>> +	uint32_t handle;
>>>>> +	uint64_t map_offset;
>>>>> +};
>>>>> +
>>>>> +struct drm_via_gem_unmap {
>>>>> +	uint32_t handle;
>>>>> +};
>>>>> +
>>>>>     #if defined(__cplusplus)
>>>>>     }
>>>>>     #endif
>>>>> --
>>>>> 2.35.1
>>>>>
>>>>
>>>> -- 
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>> (HRB 36809, AG Nürnberg)
>>>> Geschäftsführer: Ivo Totev
>>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Kevin Brace]

Hi Thomas,

I am honestly surprised of the e-mail back and forth regarding the mainlining of OpenChrome DRM, but let me state my position.
Considering the age of the hardware I am dealing with (i.e., pre-OpenGL 2.x generation 3D engine), I do not anticipate being able to run OpenChrome on Wayland with acceleration.
As a first step after mainlining, I am looking to add uAPI to pass 2D / 3D acceleration commands to the graphics engine, but frankly, I am going to focus on the 2D acceleration side first.
Considering the age of the hardware, I do not think limiting the use to X.Org X Server is a bad choice.
I do OpenChrome development on Gentoo Linux where 32-bit x86 ISA and X.Org X Server are still fully supported.
Adding 3D acceleration will likely be done after 2D and video accelerations are mostly working.
The proposed OpenChrome uAPI is essentially a cutdown version of the mostly 2D acceleration focused implementation (my opinion) being worked on and off since 2011.
The limited addition of uAPI calls is merely a preparatory step for adding 2D acceleration in the near future (I have not started the work yet.).
I assume you are aware that OpenChrome DDX is a user of DRM_VIA_GEM_CREATE, DRM_VIA_GEM_MAP, and DRM_VIA_GEM_UNMAP uAPIs.
For those who still choose to use older generation hardware, I think X.Org X Server still has a lot of life left in it, and I plan to continue modernizing the graphics stack for what I call "underserved" (i.e., neglected) graphics hardware.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Tuesday, August 02, 2022 at 4:38 AM
> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> To: "Kevin Brace" <kevinbrace@gmx.com>
> Cc: dri-devel@lists.freedesktop.org
> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>
> Hi
> 
> Am 02.08.22 um 05:45 schrieb Kevin Brace:
> > Hi Thomas,
> > 
> > I hope I am comprehending this right.
> > Yes, I am adding 3 new uAPI calls, not 6 of them.
> > Correspondingly, there are 3 new structs added.
> 
> That's understood.
> 
> > While I may drop one (unmap uAPI), I personally do not wish to drop the other two at this point.
> > Instead, I may need to add setparam and / or getparam uAPI to pass PCI Device ID back to the userspace.
> > This is mainly needed by Mesa, although there is no code for Mesa at this point.
> 
> Exactly my point! There's no userspace for it. That's why Sam and me are 
> asking you to remove all kinds if uapi changes or ioctls from the 
> patchset until Mesa (or some other component) requires it.
> 
> > I fear dropping the remaining two will require substantial redesign, and I will like to avoid this since the code is already working.
> 
> No, it won't require a redesign. You'll have to remove the changes to 
> the uapi header and any new ioctls that are in the patchset. Userspace 
> programs; such as X11's modesetting driver, Weston or Gnome; will use 
> the kernel's dumb-buffer ioctls to create unaccelerated buffers.  You 
> won't need any via-specific code in userspace. It's all there already 
> and fully driver independent. Mesa will do software rendering.  For the 
> kernel's dumb buffers, please see [1].
> 
> > It is my plan to proceed to adding acceleration after the code is added to the mainline kernel tree, so I will like to do it the way it is set up now.
> 
> You can still send the current uapi changes when you add 3d acceleration 
> to the kernel and Mesa.  But once these interfaces have been added to 
> the kernel, they are nearly impossible to change or remove. That's why 
> we don't want to do this now.
> 
> Best regards
> Thomas
> 
> [1] 
> https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#dumb-buffer-objects
> 
> > 
> > Regards,
> > 
> > Kevin Brace
> > Brace Computer Laboratory blog
> > https://bracecomputerlab.com
> > 
> > 
> >> Sent: Monday, August 01, 2022 at 11:49 AM
> >> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> >> To: "Kevin Brace" <kevinbrace@gmx.com>
> >> Cc: dri-devel@lists.freedesktop.org
> >> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
> >>
> >> Hi Kevin
> >>
> >> Am 31.07.22 um 00:48 schrieb Kevin Brace:
> >>> Hi Thomas,
> >>>
> >>> I cannot drop the older DRI1 based uAPI calls.
> >>> This is because include/uapi/drm/via_drm.h needs to retain backward compatibility with the existing OpenChrome DDX's XvMC library (it gets compiled when OpenChrome DDX is built) and likely with the existing DDX Xv code as well.
> >>> If I remove the DRI1 based uAPI calls, the XvMC library will not get compiled (compile error will occur since the XvMC library assumes the presence of DRI1 based uAPI), and I assume the same for the DDX Xv code (I cannot even reach here since the XvMC library is compiled first).
> >>> Although the v3 patch does not contain it, v4 patch will utilize drm_invalid_op() for the discontinued (not deprecated since OpenChrome DRM does not support the older DRI1 based uAPI at all) DRI1 based uAPI.
> >>>
> >>> https://cgit.freedesktop.org/openchrome/drm-openchrome/commit/?h=drm-next-5.20&id=16b3d68f95c9ccd15b7a3310e5d752fabbc76518
> >>>
> >>> drm_invalid_op() is related to drm_ioctl.c, and is meant for legacy DRMs like Radeon, i915, etc.
> >>> Since OpenChrome DRM is not a clean sheet design (related to VIA DRM to some extent), I will use this function for properly handling discontinued legacy uAPI calls.
> >>> I hope this explanation / reasoning is okay with you.
> >>
> >> I'm not sure I understand your reply ormaybe I'm just missing something
> >> here.
> >>
> >> I'm not asking you to remove the existing DRI1 uapi. I'm just asking to
> >> not add the 6 new _GEM_ defines and 3 new _gem_ structures now.  You
> >> mentioned that the driver does not yet support acceleration of any kind.
> >> So there should be no need to extend to uapi now.  You can still do this
> >> when you add acceleration to the driver.
> >>
> >> Until then, the Xorg modesetting driver or any Compositor can use the
> >> generic dumb-buffer ioctls that create buffers with no acceleration.
> >>
> >> Best regards
> >> Thomas
> >>
> >>>
> >>> Regards,
> >>>
> >>> Kevin Brace
> >>> Brace Computer Laboratory blog
> >>> https://bracecomputerlab.com
> >>>
> >>>> Sent: Tuesday, July 26, 2022 at 11:24 AM
> >>>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> >>>> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
> >>>> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
> >>>> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
> >>>>
> >>>> Hi
> >>>>
> >>>> Am 26.07.22 um 01:53 schrieb Kevin Brace:
> >>>>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> >>>>>
> >>>>> Changed the uAPI.
> >>>>>
> >>>>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> >>>>> ---
> >>>>>     include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
> >>>>>     1 file changed, 31 insertions(+), 4 deletions(-)
> >>>>>
> >>>>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
> >>>>> index a1e125d42208..e9da45ce130a 100644
> >>>>> --- a/include/uapi/drm/via_drm.h
> >>>>> +++ b/include/uapi/drm/via_drm.h
> >>>>> @@ -1,4 +1,5 @@
> >>>>>     /*
> >>>>> + * Copyright © 2020 Kevin Brace
> >>>>>      * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
> >>>>>      * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
> >>>>>      *
> >>>>> @@ -16,10 +17,10 @@
> >>>>>      * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >>>>>      * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >>>>>      * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> >>>>> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> >>>>> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >>>>> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> >>>>> - * DEALINGS IN THE SOFTWARE.
> >>>>> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
> >>>>> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
> >>>>> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
> >>>>> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> >>>>>      */
> >>>>>     #ifndef _VIA_DRM_H_
> >>>>>     #define _VIA_DRM_H_
> >>>>> @@ -81,6 +82,11 @@ extern "C" {
> >>>>>     #define DRM_VIA_DMA_BLIT        0x0e
> >>>>>     #define DRM_VIA_BLIT_SYNC       0x0f
> >>>>>
> >>>>> +#define	DRM_VIA_GEM_CREATE	0x10
> >>>>> +#define	DRM_VIA_GEM_MAP		0x11
> >>>>> +#define	DRM_VIA_GEM_UNMAP	0x12
> >>>>
> >>>> This looks a lot like ioctl ops for using accelerated HW buffers. But
> >>>> the patch is near the end of the series and you said in the series'
> >>>> cover letter that there's no acceleration. I suspect that these
> >>>> constants are currently unused?  If so, please drop the patch from the
> >>>> series. If can be merged later when something really depends on it.
> >>>>
> >>>> Best regards
> >>>> Thomas
> >>>>
> >>>>> +
> >>>>> +
> >>>>>     #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
> >>>>>     #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
> >>>>>     #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
> >>>>> @@ -97,6 +103,10 @@ extern "C" {
> >>>>>     #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
> >>>>>     #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
> >>>>>
> >>>>> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
> >>>>> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
> >>>>> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
> >>>>> +
> >>>>>     /* Indices into buf.Setup where various bits of state are mirrored per
> >>>>>      * context and per buffer.  These can be fired at the card as a unit,
> >>>>>      * or in a piecewise fashion as required.
> >>>>> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
> >>>>>     	drm_via_blitsync_t sync;
> >>>>>     } drm_via_dmablit_t;
> >>>>>
> >>>>> +struct drm_via_gem_create {
> >>>>> +	uint64_t size;
> >>>>> +	uint32_t alignment;
> >>>>> +	uint32_t domain;
> >>>>> +	uint32_t handle;
> >>>>> +	uint64_t offset;
> >>>>> +};
> >>>>> +
> >>>>> +struct drm_via_gem_map {
> >>>>> +	uint32_t handle;
> >>>>> +	uint64_t map_offset;
> >>>>> +};
> >>>>> +
> >>>>> +struct drm_via_gem_unmap {
> >>>>> +	uint32_t handle;
> >>>>> +};
> >>>>> +
> >>>>>     #if defined(__cplusplus)
> >>>>>     }
> >>>>>     #endif
> >>>>> --
> >>>>> 2.35.1
> >>>>>
> >>>>
> >>>> -- 
> >>>> Thomas Zimmermann
> >>>> Graphics Driver Developer
> >>>> SUSE Software Solutions Germany GmbH
> >>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >>>> (HRB 36809, AG Nürnberg)
> >>>> Geschäftsführer: Ivo Totev
> >>>>
> >>
> >> -- 
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Software Solutions Germany GmbH
> >> Maxfeldstr. 5, 90409 Nürnberg, Germany
> >> (HRB 36809, AG Nürnberg)
> >> Geschäftsführer: Ivo Totev
> >>
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
>

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 14640 bytes --]

Hi

Am 03.08.22 um 03:49 schrieb Kevin Brace:
> Hi Thomas,
> 
> I am honestly surprised of the e-mail back and forth regarding the mainlining of OpenChrome DRM, but let me state my position.

That's because your expectations are unrealistic. We don't know you, you 
apparently have little prior kernel experience and want to merge code 
that is essentially your pet project. It's also not a good start to 
insist on merging new uapi calls.

Upstream kernel is not a place to dump code that you declare ready. 
You're becoming part of a community. Your first priority should be to 
earn the trust of the other developers on this mailing list.

> Considering the age of the hardware I am dealing with (i.e., pre-OpenGL 2.x generation 3D engine), I do not anticipate being able to run OpenChrome on Wayland with acceleration.

Acceleration and wayland are independent from each other.

> As a first step after mainlining, I am looking to add uAPI to pass 2D / 3D acceleration commands to the graphics engine, but frankly, I am going to focus on the 2D acceleration side first.

Did you read

   https://blog.ffwll.ch/2018/08/no-2d-in-drm.html

?

> Considering the age of the hardware, I do not think limiting the use to X.Org X Server is a bad choice.

If you're not interested in running modern userspace, then why are you 
doing this anyway? Everyone with an X server can already use the 
existing DRI1 code.

You also don't seem to understand the state of the Linux graphics stack. 
First of all, you're not building a driver for one particular program, 
but for all of them. There's the X server, a number of Wayland 
compositors, and a few more outlandish things like kmscon.

And secondly, did you read

   https://ajaxnwnk.blogspot.com/2020/10/on-abandoning-x-server.html

?

Xorg-on-hardware is pretty much dead. There hasn't been a release for 
years. What will remain is Xorg-on-Wayland, which is provided by xwayland.

> I do OpenChrome development on Gentoo Linux where 32-bit x86 ISA and X.Org X Server are still fully supported.
> Adding 3D acceleration will likely be done after 2D and video accelerations are mostly working.

Video acceleration seems realistic. 2d and 3d not so much. 2d for 
reasons given in Daniel's blog. 3d because the hardware might be too old 
already. If VIA really only supports OpenGL-1.x-era features, you'd have 
to revive the old mesa branches to use it. But I can't say for sure.

> The proposed OpenChrome uAPI is essentially a cutdown version of the mostly 2D acceleration focused implementation (my opinion) being worked on and off since 2011.
> The limited addition of uAPI calls is merely a preparatory step for adding 2D acceleration in the near future (I have not started the work yet.).
> I assume you are aware that OpenChrome DDX is a user of DRM_VIA_GEM_CREATE, DRM_VIA_GEM_MAP, and DRM_VIA_GEM_UNMAP uAPIs.

To my understanding, the DDX code only exists in a repository in a git 
tree of yours. That's not "userspace".

> For those who still choose to use older generation hardware, I think X.Org X Server still has a lot of life left in it, and I plan to continue modernizing the graphics stack for what I call "underserved" (i.e., neglected) graphics hardware.

As I said we welcome drivers for old hardware. But your current proposal 
is not feasible and will cause friction with upstream.

Here's a suggestion that is more promising.

  1) Get an un-accelerated driver merged without new uapi, ioctls, etc. 
All you need is there already. We already agreed to take the code mostly 
as-is and do the cleanups later on.

  2a) Stay around on dri-devel, send improvements and fixes for the 
merged code.

  2b) In parallel, you can work on video, 3d, etc in both kernel and 
userspace.

  3) When you have a feature ready, send a patchset to dri-devel with 
the kernel changes and another patchset to the respective userspace 
project (e.g., Mesa). They should refer to each other, so that reviewers 
can see both sides of the interface at the same time.


Best regards
Thomas

> 
> Regards,
> 
> Kevin Brace
> Brace Computer Laboratory blog
> https://bracecomputerlab.com
> 
> 
>> Sent: Tuesday, August 02, 2022 at 4:38 AM
>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
>> To: "Kevin Brace" <kevinbrace@gmx.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>>
>> Hi
>>
>> Am 02.08.22 um 05:45 schrieb Kevin Brace:
>>> Hi Thomas,
>>>
>>> I hope I am comprehending this right.
>>> Yes, I am adding 3 new uAPI calls, not 6 of them.
>>> Correspondingly, there are 3 new structs added.
>>
>> That's understood.
>>
>>> While I may drop one (unmap uAPI), I personally do not wish to drop the other two at this point.
>>> Instead, I may need to add setparam and / or getparam uAPI to pass PCI Device ID back to the userspace.
>>> This is mainly needed by Mesa, although there is no code for Mesa at this point.
>>
>> Exactly my point! There's no userspace for it. That's why Sam and me are
>> asking you to remove all kinds if uapi changes or ioctls from the
>> patchset until Mesa (or some other component) requires it.
>>
>>> I fear dropping the remaining two will require substantial redesign, and I will like to avoid this since the code is already working.
>>
>> No, it won't require a redesign. You'll have to remove the changes to
>> the uapi header and any new ioctls that are in the patchset. Userspace
>> programs; such as X11's modesetting driver, Weston or Gnome; will use
>> the kernel's dumb-buffer ioctls to create unaccelerated buffers.  You
>> won't need any via-specific code in userspace. It's all there already
>> and fully driver independent. Mesa will do software rendering.  For the
>> kernel's dumb buffers, please see [1].
>>
>>> It is my plan to proceed to adding acceleration after the code is added to the mainline kernel tree, so I will like to do it the way it is set up now.
>>
>> You can still send the current uapi changes when you add 3d acceleration
>> to the kernel and Mesa.  But once these interfaces have been added to
>> the kernel, they are nearly impossible to change or remove. That's why
>> we don't want to do this now.
>>
>> Best regards
>> Thomas
>>
>> [1]
>> https://www.kernel.org/doc/html/latest/gpu/drm-kms.html#dumb-buffer-objects
>>
>>>
>>> Regards,
>>>
>>> Kevin Brace
>>> Brace Computer Laboratory blog
>>> https://bracecomputerlab.com
>>>
>>>
>>>> Sent: Monday, August 01, 2022 at 11:49 AM
>>>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
>>>> To: "Kevin Brace" <kevinbrace@gmx.com>
>>>> Cc: dri-devel@lists.freedesktop.org
>>>> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>>>>
>>>> Hi Kevin
>>>>
>>>> Am 31.07.22 um 00:48 schrieb Kevin Brace:
>>>>> Hi Thomas,
>>>>>
>>>>> I cannot drop the older DRI1 based uAPI calls.
>>>>> This is because include/uapi/drm/via_drm.h needs to retain backward compatibility with the existing OpenChrome DDX's XvMC library (it gets compiled when OpenChrome DDX is built) and likely with the existing DDX Xv code as well.
>>>>> If I remove the DRI1 based uAPI calls, the XvMC library will not get compiled (compile error will occur since the XvMC library assumes the presence of DRI1 based uAPI), and I assume the same for the DDX Xv code (I cannot even reach here since the XvMC library is compiled first).
>>>>> Although the v3 patch does not contain it, v4 patch will utilize drm_invalid_op() for the discontinued (not deprecated since OpenChrome DRM does not support the older DRI1 based uAPI at all) DRI1 based uAPI.
>>>>>
>>>>> https://cgit.freedesktop.org/openchrome/drm-openchrome/commit/?h=drm-next-5.20&id=16b3d68f95c9ccd15b7a3310e5d752fabbc76518
>>>>>
>>>>> drm_invalid_op() is related to drm_ioctl.c, and is meant for legacy DRMs like Radeon, i915, etc.
>>>>> Since OpenChrome DRM is not a clean sheet design (related to VIA DRM to some extent), I will use this function for properly handling discontinued legacy uAPI calls.
>>>>> I hope this explanation / reasoning is okay with you.
>>>>
>>>> I'm not sure I understand your reply ormaybe I'm just missing something
>>>> here.
>>>>
>>>> I'm not asking you to remove the existing DRI1 uapi. I'm just asking to
>>>> not add the 6 new _GEM_ defines and 3 new _gem_ structures now.  You
>>>> mentioned that the driver does not yet support acceleration of any kind.
>>>> So there should be no need to extend to uapi now.  You can still do this
>>>> when you add acceleration to the driver.
>>>>
>>>> Until then, the Xorg modesetting driver or any Compositor can use the
>>>> generic dumb-buffer ioctls that create buffers with no acceleration.
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>>
>>>>> Regards,
>>>>>
>>>>> Kevin Brace
>>>>> Brace Computer Laboratory blog
>>>>> https://bracecomputerlab.com
>>>>>
>>>>>> Sent: Tuesday, July 26, 2022 at 11:24 AM
>>>>>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
>>>>>> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
>>>>>> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
>>>>>> Subject: Re: [PATCH v3 26/32] drm/via: Add via_drm.h
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Am 26.07.22 um 01:53 schrieb Kevin Brace:
>>>>>>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>>>>>
>>>>>>> Changed the uAPI.
>>>>>>>
>>>>>>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>>>>> ---
>>>>>>>      include/uapi/drm/via_drm.h | 35 +++++++++++++++++++++++++++++++----
>>>>>>>      1 file changed, 31 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/uapi/drm/via_drm.h b/include/uapi/drm/via_drm.h
>>>>>>> index a1e125d42208..e9da45ce130a 100644
>>>>>>> --- a/include/uapi/drm/via_drm.h
>>>>>>> +++ b/include/uapi/drm/via_drm.h
>>>>>>> @@ -1,4 +1,5 @@
>>>>>>>      /*
>>>>>>> + * Copyright © 2020 Kevin Brace
>>>>>>>       * Copyright 1998-2003 VIA Technologies, Inc. All Rights Reserved.
>>>>>>>       * Copyright 2001-2003 S3 Graphics, Inc. All Rights Reserved.
>>>>>>>       *
>>>>>>> @@ -16,10 +17,10 @@
>>>>>>>       * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>>>>>>>       * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>>>>>>       * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
>>>>>>> - * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>>>>>>> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>>>>>>> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>>>>>> - * DEALINGS IN THE SOFTWARE.
>>>>>>> + * THE AUTHORS, COPYRIGHT HOLDERS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY
>>>>>>> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT
>>>>>>> + * OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
>>>>>>> + * THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>>>>>>>       */
>>>>>>>      #ifndef _VIA_DRM_H_
>>>>>>>      #define _VIA_DRM_H_
>>>>>>> @@ -81,6 +82,11 @@ extern "C" {
>>>>>>>      #define DRM_VIA_DMA_BLIT        0x0e
>>>>>>>      #define DRM_VIA_BLIT_SYNC       0x0f
>>>>>>>
>>>>>>> +#define	DRM_VIA_GEM_CREATE	0x10
>>>>>>> +#define	DRM_VIA_GEM_MAP		0x11
>>>>>>> +#define	DRM_VIA_GEM_UNMAP	0x12
>>>>>>
>>>>>> This looks a lot like ioctl ops for using accelerated HW buffers. But
>>>>>> the patch is near the end of the series and you said in the series'
>>>>>> cover letter that there's no acceleration. I suspect that these
>>>>>> constants are currently unused?  If so, please drop the patch from the
>>>>>> series. If can be merged later when something really depends on it.
>>>>>>
>>>>>> Best regards
>>>>>> Thomas
>>>>>>
>>>>>>> +
>>>>>>> +
>>>>>>>      #define DRM_IOCTL_VIA_ALLOCMEM	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_ALLOCMEM, drm_via_mem_t)
>>>>>>>      #define DRM_IOCTL_VIA_FREEMEM	  DRM_IOW( DRM_COMMAND_BASE + DRM_VIA_FREEMEM, drm_via_mem_t)
>>>>>>>      #define DRM_IOCTL_VIA_AGP_INIT	  DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_AGP_INIT, drm_via_agp_t)
>>>>>>> @@ -97,6 +103,10 @@ extern "C" {
>>>>>>>      #define DRM_IOCTL_VIA_DMA_BLIT    DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_DMA_BLIT, drm_via_dmablit_t)
>>>>>>>      #define DRM_IOCTL_VIA_BLIT_SYNC   DRM_IOW(DRM_COMMAND_BASE + DRM_VIA_BLIT_SYNC, drm_via_blitsync_t)
>>>>>>>
>>>>>>> +#define	DRM_IOCTL_VIA_GEM_CREATE	DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_CREATE, struct drm_via_gem_create)
>>>>>>> +#define	DRM_IOCTL_VIA_GEM_MAP		DRM_IOWR(DRM_COMMAND_BASE + DRM_VIA_GEM_MAP, struct drm_via_gem_map)
>>>>>>> +#define	DRM_IOCTL_VIA_GEM_UNMAP		DRM_IOR(DRM_COMMAND_BASE + DRM_VIA_GEM_UNMAP, struct drm_via_gem_unmap)
>>>>>>> +
>>>>>>>      /* Indices into buf.Setup where various bits of state are mirrored per
>>>>>>>       * context and per buffer.  These can be fired at the card as a unit,
>>>>>>>       * or in a piecewise fashion as required.
>>>>>>> @@ -275,6 +285,23 @@ typedef struct drm_via_dmablit {
>>>>>>>      	drm_via_blitsync_t sync;
>>>>>>>      } drm_via_dmablit_t;
>>>>>>>
>>>>>>> +struct drm_via_gem_create {
>>>>>>> +	uint64_t size;
>>>>>>> +	uint32_t alignment;
>>>>>>> +	uint32_t domain;
>>>>>>> +	uint32_t handle;
>>>>>>> +	uint64_t offset;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct drm_via_gem_map {
>>>>>>> +	uint32_t handle;
>>>>>>> +	uint64_t map_offset;
>>>>>>> +};
>>>>>>> +
>>>>>>> +struct drm_via_gem_unmap {
>>>>>>> +	uint32_t handle;
>>>>>>> +};
>>>>>>> +
>>>>>>>      #if defined(__cplusplus)
>>>>>>>      }
>>>>>>>      #endif
>>>>>>> --
>>>>>>> 2.35.1
>>>>>>>
>>>>>>
>>>>>> -- 
>>>>>> Thomas Zimmermann
>>>>>> Graphics Driver Developer
>>>>>> SUSE Software Solutions Germany GmbH
>>>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>>>> (HRB 36809, AG Nürnberg)
>>>>>> Geschäftsführer: Ivo Totev
>>>>>>
>>>>
>>>> -- 
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>> (HRB 36809, AG Nürnberg)
>>>> Geschäftsführer: Ivo Totev
>>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 26/32] drm/via: Add via_drm.h

[Sam Ravnborg]

Hi Kevin,

>  1) Get an un-accelerated driver merged without new uapi, ioctls, etc. All
> you need is there already. We already agreed to take the code mostly as-is
> and do the cleanups later on.
> 
>  2a) Stay around on dri-devel, send improvements and fixes for the merged
> code.
> 
>  2b) In parallel, you can work on video, 3d, etc in both kernel and
> userspace.

And on top of this the driver will see all the refactoring going on in
drm drivers all the time, and we may find cases where the driver can use
more of the drm helpers.

I really hope to see next revision where there is focus on the general
un-accelerated case and without the extras as Thomas mentions.

	Sam

[PATCH v3 27/32] drm/via: Add new VIA Technologies Chrome supporting PCI IDs to DRM

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 include/drm/drm_pciids.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
index b7e899ce44f0..e91b93d635fa 100644
--- a/include/drm/drm_pciids.h
+++ b/include/drm/drm_pciids.h
@@ -777,7 +777,11 @@
 	{0x1106, 0x3344, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0x1106, 0x3343, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0x1106, 0x3230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
+	{0x1106, 0x3371, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
 	{0x1106, 0x3157, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_PRO_GROUP_A}, \
+	{0x1106, 0x1122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
+	{0x1106, 0x5122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
+	{0x1106, 0x7122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
 	{0, 0, 0}

 #define i810_PCI_IDS \
--
2.35.1

[PATCH v3 28/32] drm/via: Zero out chip type field

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 include/drm/drm_pciids.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/drm/drm_pciids.h b/include/drm/drm_pciids.h
index e91b93d635fa..a4567d2918a9 100644
--- a/include/drm/drm_pciids.h
+++ b/include/drm/drm_pciids.h
@@ -770,18 +770,18 @@

 #define viadrv_PCI_IDS \
 	{0x1106, 0x3022, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
-	{0x1106, 0x3118, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_PRO_GROUP_A}, \
+	{0x1106, 0x3118, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0x1106, 0x3122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0x1106, 0x7205, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0x1106, 0x3108, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0x1106, 0x3344, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0x1106, 0x3343, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
-	{0x1106, 0x3230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
-	{0x1106, 0x3371, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
-	{0x1106, 0x3157, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_PRO_GROUP_A}, \
-	{0x1106, 0x1122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
-	{0x1106, 0x5122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
-	{0x1106, 0x7122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, VIA_DX9_0}, \
+	{0x1106, 0x3230, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
+	{0x1106, 0x3371, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
+	{0x1106, 0x3157, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
+	{0x1106, 0x1122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
+	{0x1106, 0x5122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
+	{0x1106, 0x7122, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0}, \
 	{0, 0, 0}

 #define i810_PCI_IDS \
--
2.35.1

[PATCH v3 29/32] drm/via: Add new VIA Technologies PCI device IDs related to graphics

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 include/linux/pci_ids.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 0178823ce8c2..809c61a10fe1 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1423,8 +1423,11 @@
 #define PCI_DEVICE_ID_VIA_VT3324	0x0324
 #define PCI_DEVICE_ID_VIA_VT3336	0x0336
 #define PCI_DEVICE_ID_VIA_VT3351	0x0351
+#define PCI_DEVICE_ID_VIA_VT3353	0x0353
 #define PCI_DEVICE_ID_VIA_VT3364	0x0364
 #define PCI_DEVICE_ID_VIA_8371_0	0x0391
+#define PCI_DEVICE_ID_VIA_VT3409	0x0409
+#define PCI_DEVICE_ID_VIA_VT3410	0x0410
 #define PCI_DEVICE_ID_VIA_6415		0x0415
 #define PCI_DEVICE_ID_VIA_8501_0	0x0501
 #define PCI_DEVICE_ID_VIA_82C561	0x0561
@@ -1438,6 +1441,7 @@
 #define PCI_DEVICE_ID_VIA_8605_0	0x0605
 #define PCI_DEVICE_ID_VIA_82C686	0x0686
 #define PCI_DEVICE_ID_VIA_82C691_0	0x0691
+#define PCI_DEVICE_ID_VIA_VT1122	0x1122
 #define PCI_DEVICE_ID_VIA_82C576_1	0x1571
 #define PCI_DEVICE_ID_VIA_82C586_2	0x3038
 #define PCI_DEVICE_ID_VIA_82C586_3	0x3040
@@ -1452,16 +1456,20 @@
 #define PCI_DEVICE_ID_VIA_8653_0	0x3101
 #define PCI_DEVICE_ID_VIA_8622		0x3102
 #define PCI_DEVICE_ID_VIA_8235_USB_2	0x3104
+#define PCI_DEVICE_ID_VIA_K8M800	0x3108
 #define PCI_DEVICE_ID_VIA_8233C_0	0x3109
 #define PCI_DEVICE_ID_VIA_8361		0x3112
 #define PCI_DEVICE_ID_VIA_XM266		0x3116
+#define PCI_DEVICE_ID_VIA_PM800 	0x3118
 #define PCI_DEVICE_ID_VIA_612X		0x3119
+#define PCI_DEVICE_ID_VIA_CLE266	0x3122
 #define PCI_DEVICE_ID_VIA_862X_0	0x3123
 #define PCI_DEVICE_ID_VIA_8753_0	0x3128
 #define PCI_DEVICE_ID_VIA_8233A		0x3147
 #define PCI_DEVICE_ID_VIA_8703_51_0	0x3148
 #define PCI_DEVICE_ID_VIA_8237_SATA	0x3149
 #define PCI_DEVICE_ID_VIA_XN266		0x3156
+#define PCI_DEVICE_ID_VIA_VT3157	0x3157
 #define PCI_DEVICE_ID_VIA_6410		0x3164
 #define PCI_DEVICE_ID_VIA_8754C_0	0x3168
 #define PCI_DEVICE_ID_VIA_8235		0x3177
@@ -1470,11 +1478,18 @@
 #define PCI_DEVICE_ID_VIA_8378_0	0x3205
 #define PCI_DEVICE_ID_VIA_8783_0	0x3208
 #define PCI_DEVICE_ID_VIA_8237		0x3227
+#define PCI_DEVICE_ID_VIA_K8M890	0x3230
 #define PCI_DEVICE_ID_VIA_8251		0x3287
+#define PCI_DEVICE_ID_VIA_VT3343	0x3343
+#define PCI_DEVICE_ID_VIA_CN700 	0x3344
+#define PCI_DEVICE_ID_VIA_P4M900	0x3371
 #define PCI_DEVICE_ID_VIA_8261		0x3402
 #define PCI_DEVICE_ID_VIA_8237A		0x3337
 #define PCI_DEVICE_ID_VIA_8237S		0x3372
+#define PCI_DEVICE_ID_VIA_VX875 	0x5122
 #define PCI_DEVICE_ID_VIA_SATA_EIDE	0x5324
+#define PCI_DEVICE_ID_VIA_VX900_VGA	0x7122
+#define PCI_DEVICE_ID_VIA_KM400 	0x7205
 #define PCI_DEVICE_ID_VIA_8231		0x8231
 #define PCI_DEVICE_ID_VIA_8231_4	0x8235
 #define PCI_DEVICE_ID_VIA_8365_1	0x8305
--
2.35.1

[PATCH v3 30/32] drm/via: Add Kconfig

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 drivers/gpu/drm/via/Kconfig

diff --git a/drivers/gpu/drm/via/Kconfig b/drivers/gpu/drm/via/Kconfig
new file mode 100644
index 000000000000..7c4656a1d473
--- /dev/null
+++ b/drivers/gpu/drm/via/Kconfig
@@ -0,0 +1,9 @@
+config DRM_OPENCHROME
+	tristate "OpenChrome (VIA Technologies Chrome)"
+	depends on DRM && PCI && X86
+	select DRM_KMS_HELPER
+	select DRM_TTM
+	help
+	  Choose this option if you have VIA Technologies UniChrome or
+	  Chrome9 integrated graphics. If M is selected the module will
+	  be called via.
--
2.35.1

Re: [PATCH v3 30/32] drm/via: Add Kconfig

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1527 bytes --]

Hi

Am 26.07.22 um 01:53 schrieb Kevin Brace:
> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> ---
>   drivers/gpu/drm/via/Kconfig | 9 +++++++++
>   1 file changed, 9 insertions(+)
>   create mode 100644 drivers/gpu/drm/via/Kconfig
> 
> diff --git a/drivers/gpu/drm/via/Kconfig b/drivers/gpu/drm/via/Kconfig
> new file mode 100644
> index 000000000000..7c4656a1d473
> --- /dev/null
> +++ b/drivers/gpu/drm/via/Kconfig
> @@ -0,0 +1,9 @@
> +config DRM_OPENCHROME

I would keep the driver option named DRM_VIA, as it has been so far. To 
build the existing DRI1 driver, rather introduce a new config symbol 
that enables it. The rule looks something like this:

   config DRM_VIA_DRI1
   boolean "DRI1 support"
     depends on DRM_VIA && DRM_LEGACY
     help
       Build for DRI1-based userspace drivers.

It will show up as an option if the user selects both DRM_VIA and 
DRM_LEGACY.

Best regards
Thomas


> +	tristate "OpenChrome (VIA Technologies Chrome)"
> +	depends on DRM && PCI && X86
> +	select DRM_KMS_HELPER
> +	select DRM_TTM
> +	help
> +	  Choose this option if you have VIA Technologies UniChrome or
> +	  Chrome9 integrated graphics. If M is selected the module will
> +	  be called via.
> --
> 2.35.1
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 30/32] drm/via: Add Kconfig

[Kevin Brace]

Hi Thomas,

In general, I am okay with the idea of keeping DRI1 based VIA DRM for now.
Personally, I am not that interested in keeping it, but I am not against keeping it, either.
Coming up with a scheme to keep the DRI1 based VIA DRM is sort of beyond my skill level,  so I am glad you and Sam came up with a scheme to keep it.
I think we will reuse this scheme when I finally have the time to get around adding KMS support to other similar legacy DRI1 based DRM modules.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Tuesday, July 26, 2022 at 12:28 AM
> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
> Subject: Re: [PATCH v3 30/32] drm/via: Add Kconfig
>
> Hi
> 
> Am 26.07.22 um 01:53 schrieb Kevin Brace:
> > From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > 
> > Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > ---
> >   drivers/gpu/drm/via/Kconfig | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >   create mode 100644 drivers/gpu/drm/via/Kconfig
> > 
> > diff --git a/drivers/gpu/drm/via/Kconfig b/drivers/gpu/drm/via/Kconfig
> > new file mode 100644
> > index 000000000000..7c4656a1d473
> > --- /dev/null
> > +++ b/drivers/gpu/drm/via/Kconfig
> > @@ -0,0 +1,9 @@
> > +config DRM_OPENCHROME
> 
> I would keep the driver option named DRM_VIA, as it has been so far. To 
> build the existing DRI1 driver, rather introduce a new config symbol 
> that enables it. The rule looks something like this:
> 
>    config DRM_VIA_DRI1
>    boolean "DRI1 support"
>      depends on DRM_VIA && DRM_LEGACY
>      help
>        Build for DRI1-based userspace drivers.
> 
> It will show up as an option if the user selects both DRM_VIA and 
> DRM_LEGACY.
> 
> Best regards
> Thomas
> 
> 
> > +	tristate "OpenChrome (VIA Technologies Chrome)"
> > +	depends on DRM && PCI && X86
> > +	select DRM_KMS_HELPER
> > +	select DRM_TTM
> > +	help
> > +	  Choose this option if you have VIA Technologies UniChrome or
> > +	  Chrome9 integrated graphics. If M is selected the module will
> > +	  be called via.
> > --
> > 2.35.1
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
>

Re: [PATCH v3 30/32] drm/via: Add Kconfig

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 3059 bytes --]

Hi

Am 31.07.22 um 03:49 schrieb Kevin Brace:
> Hi Thomas,
> 
> In general, I am okay with the idea of keeping DRI1 based VIA DRM for now.
> Personally, I am not that interested in keeping it, but I am not against keeping it, either.

Yeah, I see your point.  No one really wants to keep the DRI1 code, but 
there might be users out there, who still rely on it.  They usually only 
show up when things break.  It's really hard to remove anything once 
it's in the kernel.

Best regards
Thomas

> Coming up with a scheme to keep the DRI1 based VIA DRM is sort of beyond my skill level,  so I am glad you and Sam came up with a scheme to keep it.
> I think we will reuse this scheme when I finally have the time to get around adding KMS support to other similar legacy DRI1 based DRM modules.
> 
> Regards,
> 
> Kevin Brace
> Brace Computer Laboratory blog
> https://bracecomputerlab.com
> 
> 
>> Sent: Tuesday, July 26, 2022 at 12:28 AM
>> From: "Thomas Zimmermann" <tzimmermann@suse.de>
>> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
>> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
>> Subject: Re: [PATCH v3 30/32] drm/via: Add Kconfig
>>
>> Hi
>>
>> Am 26.07.22 um 01:53 schrieb Kevin Brace:
>>> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>>
>>> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
>>> ---
>>>    drivers/gpu/drm/via/Kconfig | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>    create mode 100644 drivers/gpu/drm/via/Kconfig
>>>
>>> diff --git a/drivers/gpu/drm/via/Kconfig b/drivers/gpu/drm/via/Kconfig
>>> new file mode 100644
>>> index 000000000000..7c4656a1d473
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/via/Kconfig
>>> @@ -0,0 +1,9 @@
>>> +config DRM_OPENCHROME
>>
>> I would keep the driver option named DRM_VIA, as it has been so far. To
>> build the existing DRI1 driver, rather introduce a new config symbol
>> that enables it. The rule looks something like this:
>>
>>     config DRM_VIA_DRI1
>>     boolean "DRI1 support"
>>       depends on DRM_VIA && DRM_LEGACY
>>       help
>>         Build for DRI1-based userspace drivers.
>>
>> It will show up as an option if the user selects both DRM_VIA and
>> DRM_LEGACY.
>>
>> Best regards
>> Thomas
>>
>>
>>> +	tristate "OpenChrome (VIA Technologies Chrome)"
>>> +	depends on DRM && PCI && X86
>>> +	select DRM_KMS_HELPER
>>> +	select DRM_TTM
>>> +	help
>>> +	  Choose this option if you have VIA Technologies UniChrome or
>>> +	  Chrome9 integrated graphics. If M is selected the module will
>>> +	  be called via.
>>> --
>>> 2.35.1
>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Ivo Totev
>>

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH v3 31/32] drm/via: Add Makefile

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/via/Makefile | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 drivers/gpu/drm/via/Makefile

diff --git a/drivers/gpu/drm/via/Makefile b/drivers/gpu/drm/via/Makefile
new file mode 100644
index 000000000000..73ccacb4cd11
--- /dev/null
+++ b/drivers/gpu/drm/via/Makefile
@@ -0,0 +1,26 @@
+#
+# Makefile for the drm device driver.  This driver provides support for the
+# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
+
+ccflags-y := -Iinclude/drm
+via-y := via_crtc.o \
+		via_crtc_hw.o \
+		via_cursor.o \
+		via_dac.o \
+		via_display.o \
+		via_drv.o \
+		via_encoder.o \
+		via_hdmi.o \
+		via_i2c.o \
+		via_init.o \
+		via_ioctl.o \
+		via_lvds.o \
+		via_object.o \
+		via_pll.o \
+		via_pm.o \
+		via_sii164.o \
+		via_tmds.o \
+		via_ttm.o \
+		via_vt1632.o
+
+obj-$(CONFIG_DRM_OPENCHROME)	+= via.o
--
2.35.1

Re: [PATCH v3 31/32] drm/via: Add Makefile

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1489 bytes --]

Hi

Am 26.07.22 um 01:53 schrieb Kevin Brace:
> From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>

I suggest to merge patches 30, 31 and 32 into one to make it easier to 
review.

Best regards
Thomas

> ---
>   drivers/gpu/drm/via/Makefile | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>   create mode 100644 drivers/gpu/drm/via/Makefile
> 
> diff --git a/drivers/gpu/drm/via/Makefile b/drivers/gpu/drm/via/Makefile
> new file mode 100644
> index 000000000000..73ccacb4cd11
> --- /dev/null
> +++ b/drivers/gpu/drm/via/Makefile
> @@ -0,0 +1,26 @@
> +#
> +# Makefile for the drm device driver.  This driver provides support for the
> +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> +
> +ccflags-y := -Iinclude/drm
> +via-y := via_crtc.o \
> +		via_crtc_hw.o \
> +		via_cursor.o \
> +		via_dac.o \
> +		via_display.o \
> +		via_drv.o \
> +		via_encoder.o \
> +		via_hdmi.o \
> +		via_i2c.o \
> +		via_init.o \
> +		via_ioctl.o \
> +		via_lvds.o \
> +		via_object.o \
> +		via_pll.o \
> +		via_pm.o \
> +		via_sii164.o \
> +		via_tmds.o \
> +		via_ttm.o \
> +		via_vt1632.o
> +
> +obj-$(CONFIG_DRM_OPENCHROME)	+= via.o
> --
> 2.35.1
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v3 31/32] drm/via: Add Makefile

[Kevin Brace]

Hi Thomas,

Okay, I will try to remember this.

Regards,

Kevin Brace
Brace Computer Laboratory blog
https://bracecomputerlab.com


> Sent: Tuesday, July 26, 2022 at 12:29 AM
> From: "Thomas Zimmermann" <tzimmermann@suse.de>
> To: "Kevin Brace" <kevinbrace@gmx.com>, dri-devel@lists.freedesktop.org
> Cc: "Kevin Brace" <kevinbrace@bracecomputerlab.com>
> Subject: Re: [PATCH v3 31/32] drm/via: Add Makefile
>
> Hi
> 
> Am 26.07.22 um 01:53 schrieb Kevin Brace:
> > From: Kevin Brace <kevinbrace@bracecomputerlab.com>
> > 
> > Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
> 
> I suggest to merge patches 30, 31 and 32 into one to make it easier to 
> review.
> 
> Best regards
> Thomas
> 
> > ---
> >   drivers/gpu/drm/via/Makefile | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >   create mode 100644 drivers/gpu/drm/via/Makefile
> > 
> > diff --git a/drivers/gpu/drm/via/Makefile b/drivers/gpu/drm/via/Makefile
> > new file mode 100644
> > index 000000000000..73ccacb4cd11
> > --- /dev/null
> > +++ b/drivers/gpu/drm/via/Makefile
> > @@ -0,0 +1,26 @@
> > +#
> > +# Makefile for the drm device driver.  This driver provides support for the
> > +# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
> > +
> > +ccflags-y := -Iinclude/drm
> > +via-y := via_crtc.o \
> > +		via_crtc_hw.o \
> > +		via_cursor.o \
> > +		via_dac.o \
> > +		via_display.o \
> > +		via_drv.o \
> > +		via_encoder.o \
> > +		via_hdmi.o \
> > +		via_i2c.o \
> > +		via_init.o \
> > +		via_ioctl.o \
> > +		via_lvds.o \
> > +		via_object.o \
> > +		via_pll.o \
> > +		via_pm.o \
> > +		via_sii164.o \
> > +		via_tmds.o \
> > +		via_ttm.o \
> > +		via_vt1632.o
> > +
> > +obj-$(CONFIG_DRM_OPENCHROME)	+= via.o
> > --
> > 2.35.1
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Ivo Totev
>

[PATCH v3 32/32] drm/via: Modify DRM core to be able to build OpenChrome DRM

[Kevin Brace]

From: Kevin Brace <kevinbrace@bracecomputerlab.com>

Signed-off-by: Kevin Brace <kevinbrace@bracecomputerlab.com>
---
 drivers/gpu/drm/Kconfig  | 2 ++
 drivers/gpu/drm/Makefile | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 30d5b91b717f..475d20c58da0 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -385,6 +385,8 @@ source "drivers/gpu/drm/solomon/Kconfig"

 source "drivers/gpu/drm/sprd/Kconfig"

+source "drivers/gpu/drm/via/Kconfig"
+
 config DRM_HYPERV
 	tristate "DRM Support for Hyper-V synthetic video device"
 	depends on DRM && PCI && MMU && HYPERV
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index e6d0daca9bc2..72c6db91ee61 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -97,6 +97,7 @@ obj-$(CONFIG_DRM_VC4)  += vc4/
 obj-$(CONFIG_DRM_SIS)   += sis/
 obj-$(CONFIG_DRM_SAVAGE)+= savage/
 obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/
+obj-$(CONFIG_DRM_OPENCHROME) +=via/
 obj-$(CONFIG_DRM_VGEM)	+= vgem/
 obj-$(CONFIG_DRM_VKMS)	+= vkms/
 obj-$(CONFIG_DRM_NOUVEAU) +=nouveau/
--
2.35.1