Re: [PATCH] drm/mgag200: Added support for the new device G200eH5

From: Thomas Zimmermann
Date: Thu Feb 06 2025 - 06:25:28 EST


Hi,

I want to second what Jocelyn said.


Am 05.02.25 um 23:28 schrieb Gwenael Georgeault:
- Added the new device ID
- Added new pll algorithm

Co-authored-by: Mamadou Insa Diop <mdiop@xxxxxxxxxx>
---
 drivers/gpu/drm/mgag200/Makefile          |   1 +
 drivers/gpu/drm/mgag200/mgag200_drv.c     |   4 +
 drivers/gpu/drm/mgag200/mgag200_drv.h     |   3 +
 drivers/gpu/drm/mgag200/mgag200_g200eh5.c | 202 ++++++++++++++++++++++
 4 files changed, 210 insertions(+)
 create mode 100644 drivers/gpu/drm/mgag200/mgag200_g200eh5.c

diff --git a/drivers/gpu/drm/mgag200/Makefile b/drivers/gpu/drm/mgag200/Makefile
index 5a02203fad12..94f063c8722a 100644
--- a/drivers/gpu/drm/mgag200/Makefile
+++ b/drivers/gpu/drm/mgag200/Makefile
@@ -6,6 +6,7 @@ mgag200-y := \
     mgag200_g200.o \
     mgag200_g200eh.o \
     mgag200_g200eh3.o \
+    mgag200_g200eh5.o \
     mgag200_g200er.o \
     mgag200_g200ev.o \
     mgag200_g200ew3.o \
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 069fdd2dc8f6..1c257f5b5136 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -214,6 +214,7 @@ static const struct pci_device_id mgag200_pciidlist[] = {
     { PCI_VENDOR_ID_MATROX, 0x534, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_ER },
     { PCI_VENDOR_ID_MATROX, 0x536, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EW3 },
     { PCI_VENDOR_ID_MATROX, 0x538, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH3 },
+    { PCI_VENDOR_ID_MATROX, 0x53A, PCI_ANY_ID, PCI_ANY_ID, 0, 0, G200_EH5 },

It's a personal preference, but I tend to use lower case hex values, 0x53a  would be preferable here. There might be other instances in the patch.

Apart from this and what Jocelyn noted, the patch looks good.

Best regards
Thomas

{0,}
 };

@@ -256,6 +257,9 @@ mgag200_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
     case G200_EH3:
         mdev = mgag200_g200eh3_device_create(pdev, &mgag200_driver);
         break;
+    case G200_EH5:
+        mdev = mgag200_g200eh5_device_create(pdev, &mgag200_driver);
+        break;
     case G200_ER:
         mdev = mgag200_g200er_device_create(pdev, &mgag200_driver);
         break;
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 0608fc63e588..819a7e9381e3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -196,6 +196,7 @@ enum mga_type {
     G200_EV,
     G200_EH,
     G200_EH3,
+    G200_EH5,
     G200_ER,
     G200_EW3,
 };
@@ -334,6 +335,8 @@ struct mga_device *mgag200_g200eh_device_create(struct pci_dev *pdev,
                         const struct drm_driver *drv);
 struct mga_device *mgag200_g200eh3_device_create(struct pci_dev *pdev,
                          const struct drm_driver *drv);
+struct mga_device *mgag200_g200eh5_device_create(struct pci_dev *pdev,
+                         const struct drm_driver *drv);
 struct mga_device *mgag200_g200er_device_create(struct pci_dev *pdev,
                         const struct drm_driver *drv);
 struct mga_device *mgag200_g200ew3_device_create(struct pci_dev *pdev,
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh5.c b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
new file mode 100644
index 000000000000..8cb01116ac33
--- /dev/null
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/pci.h>
+#include <linux/units.h>
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_gem_atomic_helper.h>
+#include <drm/drm_probe_helper.h>
+
+#include "mgag200_drv.h"
+
+/*
+ * PIXPLLC
+ */
+
+static int mgag200_g200eh5_pixpllc_atomic_check(struct drm_crtc *crtc,
+                        struct drm_atomic_state *new_state)
+{
+    const unsigned long long VCO_MAX = 10 * GIGA; // Hz
+    const unsigned long long VCO_MIN = 2500 * MEGA; // Hz
+    const unsigned long long PLL_FREQ_REF = 25 * MEGA; // Hz
+
+    struct drm_crtc_state *new_crtc_state = drm_atomic_get_new_crtc_state(new_state, crtc);
+    struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
+    long clock = new_crtc_state->mode.clock;
+    struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
+
+    unsigned long long fdelta = 0xFFFFFFFFFFFFFFFFULL;
+
+    u16 mult_max = (u16)(VCO_MAX / PLL_FREQ_REF); // 400 (0x190)
+    u16 mult_min = (u16)(VCO_MIN / PLL_FREQ_REF); // 100 (0x64)
+
+    u64 ftmp_delta;
+    u64 computed_fo;
+
+    u16 test_m;
+    u8 test_div_a;
+    u8 test_div_b;
+    u64 fo_hz;
+
+    u8 uc_m = 0;
+    u8 uc_n = 0;
+    u8 uc_p = 0;
+
+    fo_hz = (u64)clock * HZ_PER_KHZ;
+
+    for (test_m = mult_min; test_m <= mult_max; test_m++) { // This gives 100 <= M <= 400
+        for (test_div_a = 8; test_div_a > 0; test_div_a--) { // This gives 1 <= A <= 8
+            for (test_div_b = 1; test_div_b <= test_div_a; test_div_b++) {
+                // This gives 1 <= B <= A
+                computed_fo = (PLL_FREQ_REF * test_m) /
+                    (4 * test_div_a * test_div_b);
+
+                if (computed_fo > fo_hz)
+                    ftmp_delta = computed_fo - fo_hz;
+                else
+                    ftmp_delta = fo_hz - computed_fo;
+
+                if (ftmp_delta < fdelta) {
+                    fdelta = ftmp_delta;
+                    uc_m = (u8)(0xFF & test_m);
+                    uc_n = (u8)((0x7 & (test_div_a - 1))
+                        | (0x70 & (0x7 & (test_div_b - 1)) << 4));
+                    uc_p = (u8)(1 & (test_m >> 8));
+                }
+                if (fdelta == 0)
+                    break;
+            }
+            if (fdelta == 0)
+                break;
+        }
+        if (fdelta == 0)
+            break;
+    }
+
+    pixpllc->m = uc_m + 1;
+    pixpllc->n = uc_n + 1;
+    pixpllc->p = uc_p + 1;
+    pixpllc->s = 0;
+
+    return 0;
+    }
+
+/*
+ * Mode-setting pipeline
+ */
+
+static const struct drm_plane_helper_funcs mgag200_g200eh5_primary_plane_helper_funcs = {
+    MGAG200_PRIMARY_PLANE_HELPER_FUNCS,
+};
+
+static const struct drm_plane_funcs mgag200_g200eh5_primary_plane_funcs = {
+    MGAG200_PRIMARY_PLANE_FUNCS,
+};
+
+static const struct drm_crtc_helper_funcs mgag200_g200eh5_crtc_helper_funcs = {
+    MGAG200_CRTC_HELPER_FUNCS,
+};
+
+static const struct drm_crtc_funcs mgag200_g200eh5_crtc_funcs = {
+    MGAG200_CRTC_FUNCS,
+};
+
+static int mgag200_g200eh5_pipeline_init(struct mga_device *mdev)
+{
+    struct drm_device *dev = &mdev->base;
+    struct drm_plane *primary_plane = &mdev->primary_plane;
+    struct drm_crtc *crtc = &mdev->crtc;
+    int ret;
+
+    ret = drm_universal_plane_init(dev, primary_plane, 0,
+                       &mgag200_g200eh5_primary_plane_funcs,
+                       mgag200_primary_plane_formats,
+                       mgag200_primary_plane_formats_size,
+                       mgag200_primary_plane_fmtmods,
+                       DRM_PLANE_TYPE_PRIMARY, NULL);
+    if (ret) {
+        drm_err(dev, "drm_universal_plane_init() failed: %d\n", ret);
+        return ret;
+    }
+    drm_plane_helper_add(primary_plane, &mgag200_g200eh5_primary_plane_helper_funcs);
+    drm_plane_enable_fb_damage_clips(primary_plane);
+
+    ret = drm_crtc_init_with_planes(dev, crtc, primary_plane, NULL,
+                    &mgag200_g200eh5_crtc_funcs, NULL);
+    if (ret) {
+        drm_err(dev, "drm_crtc_init_with_planes() failed: %d\n", ret);
+        return ret;
+    }
+
+    drm_crtc_helper_add(crtc, &mgag200_g200eh5_crtc_helper_funcs);
+
+    /* FIXME: legacy gamma tables, but atomic gamma doesn't work without */
+    drm_mode_crtc_set_gamma_size(crtc, MGAG200_LUT_SIZE);
+    drm_crtc_enable_color_mgmt(crtc, 0, false, MGAG200_LUT_SIZE);
+    ret = mgag200_vga_bmc_output_init(mdev);
+
+    if (ret)
+        return ret;
+
+    return 0;
+}
+
+/*
+ * DRM device
+ */
+
+static const struct mgag200_device_info mgag200_g200eh5_device_info =
+    MGAG200_DEVICE_INFO_INIT(2048, 2048, 0, false, 1, 0, false);
+
+static const struct mgag200_device_funcs mgag200_g200eh5_device_funcs = {
+    .pixpllc_atomic_check = mgag200_g200eh5_pixpllc_atomic_check,
+    .pixpllc_atomic_update = mgag200_g200eh_pixpllc_atomic_update, // same as G200EH
+};
+
+struct mga_device *mgag200_g200eh5_device_create(struct pci_dev *pdev,
+                         const struct drm_driver *drv)
+{
+    struct mga_device *mdev;
+    struct drm_device *dev;
+    resource_size_t vram_available;
+    int ret;
+
+    mdev = devm_drm_dev_alloc(&pdev->dev, drv, struct mga_device, base);
+
+    if (IS_ERR(mdev))
+        return mdev;
+    dev = &mdev->base;
+
+    pci_set_drvdata(pdev, dev);
+
+    ret = mgag200_init_pci_options(pdev, 0x00000120, 0x0000b000);
+    if (ret)
+        return ERR_PTR(ret);
+
+    ret = mgag200_device_preinit(mdev);
+    if (ret)
+        return ERR_PTR(ret);
+
+    ret = mgag200_device_init(mdev, &mgag200_g200eh5_device_info,
+                  &mgag200_g200eh5_device_funcs);
+
+    if (ret)
+        return ERR_PTR(ret);
+
+    mgag200_g200eh_init_registers(mdev); // same as G200EH
+    vram_available = mgag200_device_probe_vram(mdev);
+
+    ret = mgag200_mode_config_init(mdev, vram_available);
+    if (ret)
+        return ERR_PTR(ret);
+
+    ret = mgag200_g200eh5_pipeline_init(mdev);
+    if (ret)
+        return ERR_PTR(ret);
+
+    drm_mode_config_reset(dev);
+    drm_kms_helper_poll_init(dev);
+
+    return mdev;
+}

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)