* [PATCH] drm/mgag200: Added support for the new device G200eH5
@ 2025-02-07 19:04 Gwenael Georgeault
2025-02-11 10:53 ` Jocelyn Falempe
0 siblings, 1 reply; 11+ messages in thread
From: Gwenael Georgeault @ 2025-02-07 19:04 UTC (permalink / raw)
To: Dave Airlie, Thomas Zimmermann, Jocelyn Falempe,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel, mgi_lnx
- Added the new device ID
- Added new pll algorithm
Signed-off-by: Gwenael Georgeault <ggeorgea@matrox.com>
Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
---
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 | 205 ++++++++++++++++++++++
4 files changed, 213 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..32cd8ac018c0 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 },
{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..2c4265293504
--- /dev/null
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/limits.h>
+#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 = ULLONG_MAX;
+
+ 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;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/mgag200: Added support for the new device G200eH5
2025-02-07 19:04 [PATCH] drm/mgag200: Added support for the new device G200eH5 Gwenael Georgeault
@ 2025-02-11 10:53 ` Jocelyn Falempe
0 siblings, 0 replies; 11+ messages in thread
From: Jocelyn Falempe @ 2025-02-11 10:53 UTC (permalink / raw)
To: Gwenael Georgeault, Dave Airlie, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel, mgi_lnx
On 07/02/2025 20:04, Gwenael Georgeault wrote:
> - Added the new device ID
> - Added new pll algorithm
I pushed it to drm-misc/drm-misc-next.
Thanks
--
Jocelyn
>
> Signed-off-by: Gwenael Georgeault <ggeorgea@matrox.com>
> Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
> ---
> 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 | 205 ++++++++++++++++++++++
> 4 files changed, 213 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..32cd8ac018c0 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 },
> {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..2c4265293504
> --- /dev/null
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
> @@ -0,0 +1,204 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/limits.h>
> +#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 = ULLONG_MAX;
> +
> + 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;
> +}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/mgag200: Added support for the new device G200eH5
@ 2025-02-07 13:21 Gwenael Georgeault
2025-02-07 13:36 ` Thomas Zimmermann
0 siblings, 1 reply; 11+ messages in thread
From: Gwenael Georgeault @ 2025-02-07 13:21 UTC (permalink / raw)
To: Dave Airlie, Thomas Zimmermann, Jocelyn Falempe,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel, mgi_lnx
- Added the new device ID
- Added new pll algorithm
Signed-off-by: Gwenael Georgeault <ggeorgea@matrox.com>
Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
---
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 | 205 ++++++++++++++++++++++
4 files changed, 213 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..32cd8ac018c0 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 },
{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..2c4265293504
--- /dev/null
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <limits.h>
+
+#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 = ULLONG_MAX;
+
+ 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;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/mgag200: Added support for the new device G200eH5
2025-02-07 13:21 Gwenael Georgeault
@ 2025-02-07 13:36 ` Thomas Zimmermann
2025-02-07 14:26 ` Jocelyn Falempe
0 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2025-02-07 13:36 UTC (permalink / raw)
To: Gwenael Georgeault, Dave Airlie, Jocelyn Falempe,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel, mgi_lnx
Hi
Am 07.02.25 um 14:21 schrieb Gwenael Georgeault:
> - Added the new device ID
> - Added new pll algorithm
>
> Signed-off-by: Gwenael Georgeault <ggeorgea@matrox.com>
> Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
There's a remark below, but it might not be an issue.
> ---
> 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 | 205 ++++++++++++++++++++++
> 4 files changed, 213 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..32cd8ac018c0 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 },
> {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..2c4265293504
> --- /dev/null
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
> @@ -0,0 +1,205 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <limits.h>
I'm skeptical of this line, as it looks like it pulls in a system
header. But other drivers do the same, so it's OK, I guess.
Best regards
Thomas
> +
> +#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 = ULLONG_MAX;
> +
> + 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)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drm/mgag200: Added support for the new device G200eH5
2025-02-07 13:36 ` Thomas Zimmermann
@ 2025-02-07 14:26 ` Jocelyn Falempe
0 siblings, 0 replies; 11+ messages in thread
From: Jocelyn Falempe @ 2025-02-07 14:26 UTC (permalink / raw)
To: Thomas Zimmermann, Gwenael Georgeault, Dave Airlie,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel, mgi_lnx
On 07/02/2025 14:36, Thomas Zimmermann wrote:
> Hi
>
>
> Am 07.02.25 um 14:21 schrieb Gwenael Georgeault:
>> - Added the new device ID
>> - Added new pll algorithm
>>
>> Signed-off-by: Gwenael Georgeault <ggeorgea@matrox.com>
>> Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> There's a remark below, but it might not be an issue.
With the "#include <limits.h>" changed to "#include <linux/limits.h>"
(see below):
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
>
>> ---
>> 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 | 205 ++++++++++++++++++++++
>> 4 files changed, 213 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..32cd8ac018c0 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 },
>> {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..2c4265293504
>> --- /dev/null
>> +++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
>> @@ -0,0 +1,205 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include <limits.h>
>
> I'm skeptical of this line, as it looks like it pulls in a system
> header. But other drivers do the same, so it's OK, I guess.
Good catch, most other drivers include <linux/limits.h>, which also have
ULLONG_MAX defined through include/vdso/limits.h:
https://elixir.bootlin.com/linux/v6.13.1/source/include/linux/limits.h
https://elixir.bootlin.com/linux/v6.13.1/source/include/vdso/limits.h
Best regards,
Jocelyn
>
> Best regards
> Thomas
>
>> +
>> +#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 = ULLONG_MAX;
>> +
>> + 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;
>> +}
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/mgag200: Added support for the new device G200eH5
@ 2025-02-05 22:28 Gwenael Georgeault
2025-02-06 11:09 ` Jocelyn Falempe
2025-02-06 11:24 ` Thomas Zimmermann
0 siblings, 2 replies; 11+ messages in thread
From: Gwenael Georgeault @ 2025-02-05 22:28 UTC (permalink / raw)
To: Dave Airlie, Thomas Zimmermann, Jocelyn Falempe,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel
- Added the new device ID
- Added new pll algorithm
Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
---
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 },
{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;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/mgag200: Added support for the new device G200eH5
2025-02-05 22:28 Gwenael Georgeault
@ 2025-02-06 11:09 ` Jocelyn Falempe
2025-02-06 11:24 ` Thomas Zimmermann
1 sibling, 0 replies; 11+ messages in thread
From: Jocelyn Falempe @ 2025-02-06 11:09 UTC (permalink / raw)
To: Gwenael Georgeault, Dave Airlie, Thomas Zimmermann,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel
Thanks a lot for this rework.
When sending a new version of your patch, you can use "git send-email
-v2" to show that it's the second version of this patch.
On 05/02/2025 23:28, Gwenael Georgeault wrote:
> - Added the new device ID
> - Added new pll algorithm
>
> Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
You're missing your Signed-off-by line.
You can use git commit -s --amend to add it automatically
Also there are some spurious spaces added to your patch context, which
makes git am failed. I just replaced all occurences of "0x0A 0x20 0x20"
with "0x0A 0x20" and that fixed it.
I still have a few comments below:
> ---
> 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 },
> {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
There should be an empty line between the license and the first include
statement.
> +#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;
You can use ULLONG_MAX here instead.
> +
> + 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;
> +}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drm/mgag200: Added support for the new device G200eH5
2025-02-05 22:28 Gwenael Georgeault
2025-02-06 11:09 ` Jocelyn Falempe
@ 2025-02-06 11:24 ` Thomas Zimmermann
1 sibling, 0 replies; 11+ messages in thread
From: Thomas Zimmermann @ 2025-02-06 11:24 UTC (permalink / raw)
To: Gwenael Georgeault, Dave Airlie, Jocelyn Falempe,
Maarten Lankhorst, Maxime Ripard, David Airlie, Simona Vetter,
linux-kernel, dri-devel
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@matrox.com>
> ---
> 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)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/mgag200: Added support for the new device G200eH5
@ 2025-02-03 13:07 Gwenael Georgeault
2025-02-03 13:49 ` Jocelyn Falempe
2025-02-04 8:55 ` Thomas Zimmermann
0 siblings, 2 replies; 11+ messages in thread
From: Gwenael Georgeault @ 2025-02-03 13:07 UTC (permalink / raw)
To: David Airlie, Jocelyn Falempe, Thomas Zimmermann, dri-devel,
linux-kernel
- Added the new device ID
- Added new pll algorithm
Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
---
drivers/gpu/drm/mgag200/Makefile | 1 +
drivers/gpu/drm/mgag200/mgag200_drv.c | 4 +
drivers/gpu/drm/mgag200/mgag200_drv.h | 7 +-
drivers/gpu/drm/mgag200/mgag200_g200eh5.c | 212 ++++++++++++++++++++++
4 files changed, 222 insertions(+), 2 deletions(-)
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 },
{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..065ba09d109b 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,
};
@@ -333,11 +334,13 @@ void mgag200_g200eh_pixpllc_atomic_update(struct drm_crtc *crtc, struct drm_atom
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);
+ 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,
- const struct drm_driver *drv);
+ const struct drm_driver *drv);
/*
* mgag200_mode.c
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh5.c b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
new file mode 100644
index 000000000000..5e39504785d8
--- /dev/null
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/pci.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)
+{
+
+ static u64 ulVCOMax = 10000000000ULL; // units in Hz (10 GHz)
+ static u64 ulVCOMin = 2500000000LL; // units in Hz (2.5 GHz)
+ static u64 ulPLLFreqRef = 25000000ULL; // units in Hz (25 MHz)
+
+ 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;
+
+ u64 ulFDelta = 0xFFFFFFFFFFFFFFFFULL;
+
+ u16 ulMultMax = (u16)(ulVCOMax / ulPLLFreqRef); // 400 (0x190)
+ u16 ulMultMin = (u16)(ulVCOMin / ulPLLFreqRef); // 100 (0x64)
+
+ u64 ulFTmpDelta;
+ u64 ulComputedFo;
+
+ u16 ulTestM;
+ u8 ulTestDivA;
+ u8 ulTestDivB;
+ u64 ulFoHz;
+ int iDone = 0;
+
+ u8 ucM = 0;
+ u8 ucN = 0;
+ u8 ucP = 0;
+
+ ulFoHz = (u64)clock * 1000ULL;
+
+
+ for (ulTestM = ulMultMin; ulTestM <= ulMultMax; ulTestM++) { // This gives 100 <= M <= 400
+ for (ulTestDivA = 8; ulTestDivA > 0; ulTestDivA--) { // This gives 1 <= A <= 8
+ for (ulTestDivB = 1; ulTestDivB <= ulTestDivA; ulTestDivB++) {
+ // This gives 1 <= B <= A
+ ulComputedFo = (ulPLLFreqRef * ulTestM) /
+ (4 * ulTestDivA * ulTestDivB);
+
+ if (ulComputedFo > ulFoHz)
+ ulFTmpDelta = ulComputedFo - ulFoHz;
+ else
+ ulFTmpDelta = ulFoHz - ulComputedFo;
+
+ if (ulFTmpDelta < ulFDelta) {
+ ulFDelta = ulFTmpDelta;
+ ucM = (u8)(0xFF & ulTestM);
+ ucN = (u8)(
+ (0x7 & (ulTestDivA - 1))
+ | (0x70 & (0x7 & (ulTestDivB - 1)) << 4)
+ );
+ ucP = (u8)(1 & (ulTestM >> 8));
+ }
+ if (ulFDelta == 0) {
+ iDone = 1;
+ break;
+ }
+ } // End of DivB if (iDone)
+ if (iDone)
+ break;
+ } // End of DivA Loop
+
+ if (iDone)
+ break;
+ } // End of M Loop
+
+ pixpllc->m = ucM + 1;
+ pixpllc->n = ucN + 1;
+ pixpllc->p = ucP + 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;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drm/mgag200: Added support for the new device G200eH5
2025-02-03 13:07 Gwenael Georgeault
@ 2025-02-03 13:49 ` Jocelyn Falempe
2025-02-04 8:55 ` Thomas Zimmermann
1 sibling, 0 replies; 11+ messages in thread
From: Jocelyn Falempe @ 2025-02-03 13:49 UTC (permalink / raw)
To: Gwenael Georgeault, David Airlie, Thomas Zimmermann, dri-devel,
linux-kernel
On 03/02/2025 14:07, Gwenael Georgeault wrote:
> - Added the new device ID
> - Added new pll algorithm
Hi,
Thanks for this patch.
Overall it looks good, I have a few comments below:
Best regards,
--
Jocelyn
>
> Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
> ---
> drivers/gpu/drm/mgag200/Makefile | 1 +
> drivers/gpu/drm/mgag200/mgag200_drv.c | 4 +
> drivers/gpu/drm/mgag200/mgag200_drv.h | 7 +-
> drivers/gpu/drm/mgag200/mgag200_g200eh5.c | 212 ++++++++++++++++++++++
> 4 files changed, 222 insertions(+), 2 deletions(-)
> 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 },
> {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..065ba09d109b 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,
> };
> @@ -333,11 +334,13 @@ void mgag200_g200eh_pixpllc_atomic_update(struct
> drm_crtc *crtc, struct drm_atom
> 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);
> + const struct drm_driver *drv);
The alignment was correct, don't modify indentation on lines you don't
change.
> +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,
> - const struct drm_driver *drv);
> + const struct drm_driver *drv);
Here too
>
> /*
> * mgag200_mode.c
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh5.c b/drivers/gpu/
> drm/mgag200/mgag200_g200eh5.c
> new file mode 100644
> index 000000000000..5e39504785d8
> --- /dev/null
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/pci.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)
> +{
> +
> + static u64 ulVCOMax = 10000000000ULL; // units in Hz (10 GHz)
> + static u64 ulVCOMin = 2500000000LL; // units in Hz (2.5 GHz)
> + static u64 ulPLLFreqRef = 25000000ULL; // units in Hz (25 MHz)
We don't use type prefix in the kernel, and usually variables are all
lowercase.
Also those 3 variables don't need to be static. (Used like this, it
means it will keep it's value when you call this function again, which
is not needed as they are constant).
> +
> + 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;
> +
> + u64 ulFDelta = 0xFFFFFFFFFFFFFFFFULL;
> +
> + u16 ulMultMax = (u16)(ulVCOMax / ulPLLFreqRef); // 400 (0x190)
> + u16 ulMultMin = (u16)(ulVCOMin / ulPLLFreqRef); // 100 (0x64)
Here the type prefix is wrong, as it's not an unsigned long. But like
all variables, just remove the prefix, and rename them to lowercase
(either multmax or mult_max).
> +
> + u64 ulFTmpDelta;
> + u64 ulComputedFo;
> +
> + u16 ulTestM;
> + u8 ulTestDivA;
> + u8 ulTestDivB;
> + u64 ulFoHz;
> + int iDone = 0;
iDone is useless, if you find the right parameters, and ulFDelta is 0,
then you won't find a better solution, because you can't have
(ulFTmpDelta < ulFDelta).
It's a small optimisation to avoid some loops, but the PLL are
configured only once, so it's really not worth it.
> +
> + u8 ucM = 0;
> + u8 ucN = 0;
> + u8 ucP = 0;
> +
> + ulFoHz = (u64)clock * 1000ULL;
> +
> +
> + for (ulTestM = ulMultMin; ulTestM <= ulMultMax; ulTestM++) { //
> This gives 100 <= M <= 400
> + for (ulTestDivA = 8; ulTestDivA > 0; ulTestDivA--) { // This
> gives 1 <= A <= 8
> + for (ulTestDivB = 1; ulTestDivB <= ulTestDivA; ulTestDivB++) {
> + // This gives 1 <= B <= A
> + ulComputedFo = (ulPLLFreqRef * ulTestM) /
> + (4 * ulTestDivA * ulTestDivB);
> +
> + if (ulComputedFo > ulFoHz)
> + ulFTmpDelta = ulComputedFo - ulFoHz;
> + else
> + ulFTmpDelta = ulFoHz - ulComputedFo;
> +
> + if (ulFTmpDelta < ulFDelta) {
> + ulFDelta = ulFTmpDelta;
> + ucM = (u8)(0xFF & ulTestM);
> + ucN = (u8)(
> + (0x7 & (ulTestDivA - 1))
> + | (0x70 & (0x7 & (ulTestDivB - 1)) << 4)
> + );
> + ucP = (u8)(1 & (ulTestM >> 8));
> + }
> + if (ulFDelta == 0) {
> + iDone = 1;
> + break;
> + }
> + } // End of DivB if (iDone)
> + if (iDone)
> + break;
> + } // End of DivA Loop
> +
> + if (iDone)
> + break;
> + } // End of M Loop
> +
> + pixpllc->m = ucM + 1;
> + pixpllc->n = ucN + 1;
> + pixpllc->p = ucP + 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;
> +}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drm/mgag200: Added support for the new device G200eH5
2025-02-03 13:07 Gwenael Georgeault
2025-02-03 13:49 ` Jocelyn Falempe
@ 2025-02-04 8:55 ` Thomas Zimmermann
1 sibling, 0 replies; 11+ messages in thread
From: Thomas Zimmermann @ 2025-02-04 8:55 UTC (permalink / raw)
To: Gwenael Georgeault, David Airlie, Jocelyn Falempe, dri-devel,
linux-kernel
Hi
Am 03.02.25 um 14:07 schrieb Gwenael Georgeault:
Thanks for sending this patch and welcome to the kernel community
> - Added the new device ID
> - Added new pll algorithm
>
> Co-authored-by: Mamadou Insa Diop <mdiop@matrox.com>
> ---
> drivers/gpu/drm/mgag200/Makefile | 1 +
> drivers/gpu/drm/mgag200/mgag200_drv.c | 4 +
> drivers/gpu/drm/mgag200/mgag200_drv.h | 7 +-
> drivers/gpu/drm/mgag200/mgag200_g200eh5.c | 212 ++++++++++++++++++++++
Great, all new code located in a single file. That's how it is intended.
The code looks correct, but I have plenty of comments on the style.
> 4 files changed, 222 insertions(+), 2 deletions(-)
> 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 },
> {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..065ba09d109b 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,
> };
> @@ -333,11 +334,13 @@ void mgag200_g200eh_pixpllc_atomic_update(struct
> drm_crtc *crtc, struct drm_atom
> 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);
> + 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,
> - const struct drm_driver *drv);
> + const struct drm_driver *drv);
>
> /*
> * mgag200_mode.c
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
> b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
> new file mode 100644
> index 000000000000..5e39504785d8
> --- /dev/null
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200eh5.c
The coding style in this file is off. Jocelyn already pointed out quite
a few issues. Please run checkpatch.pl on the patch file before
submitting and fix the errors and warnings. It's in the scripts/ directory.
./scripts/checkpatch.pl --strict <filename>
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0-only
Empty line here.
> +#include <linux/pci.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)
> +{
> +
No empty line here.
> + static u64 ulVCOMax = 10000000000ULL; // units in Hz (10 GHz)
> + static u64 ulVCOMin = 2500000000LL; // units in Hz (2.5 GHz)
> + static u64 ulPLLFreqRef = 25000000ULL; // units in Hz (25 MHz)
The camel case and hungarian notation is not used within the kernel. The
code that uses this has likely been copied from somewhere else.
Fixed-size types (e.g., u64) should be avoided IIRC; unless they are
necessary. Using 'static' is ok if you also make it 'const', so that
there are real constant. For these numbers, you could also look at
<linux/units.h> for SI multipliers.
For these constants, I'd write something like
static const unsigned long long VCO_MAX = 10 * GIGA // Hz
static const unsigned long long VCO_MAX = 2500 * MEGA // Hz
static 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);
This file appears to be using 4 spaces per level of indention. Indention
within the kernel is 1 tab. Each tab is equivalent to 8 space.
> + struct mgag200_crtc_state *new_mgag200_crtc_state =
> to_mgag200_crtc_state(new_crtc_state);
> + long clock = new_crtc_state->mode.clock;
Multiple spaces after 'long'. There are similar cases below.
> + struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
> +
> + u64 ulFDelta = 0xFFFFFFFFFFFFFFFFULL;
> +
> + u16 ulMultMax = (u16)(ulVCOMax / ulPLLFreqRef); // 400 (0x190)
> + u16 ulMultMin = (u16)(ulVCOMin / ulPLLFreqRef); // 100 (0x64)
> +
> + u64 ulFTmpDelta;
> + u64 ulComputedFo;
> +
> + u16 ulTestM;
> + u8 ulTestDivA;
> + u8 ulTestDivB;
> + u64 ulFoHz;
> + int iDone = 0;
> +
> + u8 ucM = 0;
> + u8 ucN = 0;
> + u8 ucP = 0;
> +
> + ulFoHz = (u64)clock * 1000ULL;
For such conversions, you should use HZ_PER_KHZ, also found in
<linux/units.h>. Makes it clear what it does.
> +
> +
> + for (ulTestM = ulMultMin; ulTestM <= ulMultMax; ulTestM++) { //
> This gives 100 <= M <= 400
> + for (ulTestDivA = 8; ulTestDivA > 0; ulTestDivA--) { // This
> gives 1 <= A <= 8
> + for (ulTestDivB = 1; ulTestDivB <= ulTestDivA;
> ulTestDivB++) {
> + // This gives 1 <= B <= A
> + ulComputedFo = (ulPLLFreqRef * ulTestM) /
> + (4 * ulTestDivA * ulTestDivB);
> +
> + if (ulComputedFo > ulFoHz)
> + ulFTmpDelta = ulComputedFo - ulFoHz;
> + else
> + ulFTmpDelta = ulFoHz - ulComputedFo;
> +
> + if (ulFTmpDelta < ulFDelta) {
> + ulFDelta = ulFTmpDelta;
> + ucM = (u8)(0xFF & ulTestM);
> + ucN = (u8)(
> + (0x7 & (ulTestDivA - 1))
> + | (0x70 & (0x7 & (ulTestDivB - 1)) << 4)
> + );
> + ucP = (u8)(1 & (ulTestM >> 8));
> + }
> + if (ulFDelta == 0) {
> + iDone = 1;
> + break;
> + }
> + } // End of DivB if (iDone)
> + if (iDone)
> + break;
> + } // End of DivA Loop
> +
> + if (iDone)
> + break;
> + } // End of M Loop
As with all other models, frequency calculation is not easily
understandable. I haven't found a way to make these nested loops
readable, so it's OK to do this here as well.
But you should remove these '// End of' comments. This is something that
the formating should make obvious.
> +
> + pixpllc->m = ucM + 1;
> + pixpllc->n = ucN + 1;
> + pixpllc->p = ucP + 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);
This should be indented as shown at
https://elixir.bootlin.com/linux/v6.13.1/source/Documentation/process/coding-style.rst#L497
The file will tell you how to format the code.
Best regards
Thomas
> + 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)
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-02-11 10:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-07 19:04 [PATCH] drm/mgag200: Added support for the new device G200eH5 Gwenael Georgeault
2025-02-11 10:53 ` Jocelyn Falempe
-- strict thread matches above, loose matches on Subject: below --
2025-02-07 13:21 Gwenael Georgeault
2025-02-07 13:36 ` Thomas Zimmermann
2025-02-07 14:26 ` Jocelyn Falempe
2025-02-05 22:28 Gwenael Georgeault
2025-02-06 11:09 ` Jocelyn Falempe
2025-02-06 11:24 ` Thomas Zimmermann
2025-02-03 13:07 Gwenael Georgeault
2025-02-03 13:49 ` Jocelyn Falempe
2025-02-04 8:55 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox