From: Sam Ravnborg <sam@ravnborg.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
Cc: airlied@linux.ie, airlied@redhat.com, jfalempe@redhat.com,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 02/14] drm/mgag200: Move DAC-register setup into model-specific code
Date: Fri, 8 Jul 2022 17:54:31 +0200 [thread overview]
Message-ID: <YshTN7eZFTdAka/E@ravnborg.org> (raw)
In-Reply-To: <20220708093929.4446-3-tzimmermann@suse.de>
Hi Thomas,
On Fri, Jul 08, 2022 at 11:39:17AM +0200, Thomas Zimmermann wrote:
> Provide an init function for each model's DAC registers. Remove
> the shared helper.
>
> The code for initializing the DAC registers consisted of a large
> table of default value, plus many exceptions for the various G200
> models. Providing a per-model implementation makes if more readable.
The suggested implementation introduces a lot of duplications which is
not nice and the increase in readability is low as it continues to use a
lot of direct value rather than referring to register index values.
In some places it replaces nice register index values with hardcoded
constants.
I typed up something that I see as easier to read - but it is also
much more verbose.
I noticed that for example index 0x1c has the value 0xBF - but the value
is never used. With the scheme below we would only assign values to
registers in use. This would be obvious in the scheme below.
I suggest to keep the current implementation as it is easier to read
what happens with the registers, or go for a more verbose version where
there is no hard coded index to registers like for example outlined below.
Sam
struct dac_data {
u8 data;
bool use; /* If
};
All the registers that has the same values for all models:
static void mgag200_default_regs(struct dac_data *dac)
{
memset(dac, 0, MGA200_REGS); // New constat equal 0x50
dac[MGA1064_VREF_CTL].use = true;
dac[MGA1064_VREF_CTL].data = 0;
dac[MGA1064_MUL_CTL].use = true;
dac[MGA1064_MUL_CTL].data = 0;
dac[MGA1064_PIX_CLK_CTL].use = true;
dac[MGA1064_PIX_CLK_CTL] = 0xC9;
And so on.
Much more verbose than a table, but also more readable.
A generic write function:
void mgag200_write_dac(struct dac_data *dac)
{
int i;
for (i = 0; i < MGA200_REGS) {
if (dac[i].use])
WREG_DAC(i, dac[i]);
}
}
Then for each model something like
void mgag200_g200eh_init_registers(struct mga_device *mdev)
{
struct dac_data dac[MGA200_REGS];
mgag200_default_regs(&dac];
dac[MGA1064_MISC_CTL].use = true;
dac[MGA1064_MISC_CTL].data = MGA1064_MISC_CTL_VGA8 | /* G200EH specific: MGA1064_MISC_CTL */
MGA1064_MISC_CTL_DAC_RAM_CS;
/* No PIX_PLL setup */ -- The logic should be reversed I hink, so
the ones that need PLL setup set to true.
But the idea is obvious so I did not
redo it
dac[MGA1064_PIX_PLLA_M].use = false;
dac[MGA1064_PIX_PLLA_N].use = false;
dac[MGA1064_PIX_PLLA_P].use = false;
dac[MGA1064_PIX_PLLB_M].use = false;
dac[MGA1064_PIX_PLLB_N].use = false;
dac[MGA1064_PIX_PLLB_P].use = false;
dac[MGA1064_PIX_PLLC_M].use = false;
dac[MGA1064_PIX_PLLC_N].use = false;
dac[MGA1064_PIX_PLLC_P].use = false;
mgag200_write_dac(&dac);
}
> At some point, some of the initialization should probably move into
> the modesetting code.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/mgag200/mgag200_drv.h | 3 +
> drivers/gpu/drm/mgag200/mgag200_g200.c | 37 +++++++++++
> drivers/gpu/drm/mgag200/mgag200_g200eh.c | 36 ++++++++++
> drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 2 +
> drivers/gpu/drm/mgag200/mgag200_g200er.c | 34 ++++++++++
> drivers/gpu/drm/mgag200/mgag200_g200ev.c | 40 ++++++++++++
> drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 2 +
> drivers/gpu/drm/mgag200/mgag200_g200se.c | 45 +++++++++++++
> drivers/gpu/drm/mgag200/mgag200_g200wb.c | 34 ++++++++++
> drivers/gpu/drm/mgag200/mgag200_mode.c | 80 +----------------------
> 10 files changed, 234 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index 301c4ab46539..6a2a1ad914c1 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -295,10 +295,12 @@ struct mga_device *mgag200_g200_device_create(struct pci_dev *pdev, const struct
> enum mga_type type);
> struct mga_device *mgag200_g200se_device_create(struct pci_dev *pdev, const struct drm_driver *drv,
> enum mga_type type);
> +void mgag200_g200wb_init_registers(struct mga_device *mdev);
> struct mga_device *mgag200_g200wb_device_create(struct pci_dev *pdev, const struct drm_driver *drv,
> enum mga_type type);
> struct mga_device *mgag200_g200ev_device_create(struct pci_dev *pdev, const struct drm_driver *drv,
> enum mga_type type);
> +void mgag200_g200eh_init_registers(struct mga_device *mdev);
> struct mga_device *mgag200_g200eh_device_create(struct pci_dev *pdev, const struct drm_driver *drv,
> enum mga_type type);
> struct mga_device *mgag200_g200eh3_device_create(struct pci_dev *pdev, const struct drm_driver *drv,
> @@ -310,6 +312,7 @@ struct mga_device *mgag200_g200ew3_device_create(struct pci_dev *pdev, const str
>
> /* mgag200_mode.c */
> resource_size_t mgag200_device_probe_vram(struct mga_device *mdev);
> +void mgag200_init_registers(struct mga_device *mdev);
> int mgag200_modeset_init(struct mga_device *mdev, resource_size_t vram_fb_available);
>
> /* mgag200_i2c.c */
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200.c b/drivers/gpu/drm/mgag200/mgag200_g200.c
> index 674385921b7f..c6303fcb8fe7 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200.c
> @@ -30,6 +30,41 @@ static int mgag200_g200_init_pci_options(struct pci_dev *pdev)
> return mgag200_init_pci_options(pdev, option, 0x00008000);
> }
>
> +static void mgag200_g200_init_registers(struct mgag200_g200_device *g200)
> +{
> + static const u8 dacvalue[] = {
> + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
> + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + /* 0x28: */ 0x00, 0x00, 0x00, 0x00,
> + /* 0x2c: */ 0x04, /* G200 specific: MGA1064_SYS_PLL_M */
> + /* 0x2d: */ 0x2d, /* G200 specific: MGA1064_SYS_PLL_N */
> + /* 0x2e: */ 0x19, /* G200 specific: MGA1064_SYS_PLL_P */
> + /* 0x2f: */ 0x40,
> + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0
> + };
> +
> + struct mga_device *mdev = &g200->base;
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(dacvalue); ++i) {
> + if ((i <= 0x17) ||
> + (i == 0x1b) ||
> + (i == 0x1c) ||
> + ((i >= 0x1f) && (i <= 0x29)) ||
> + ((i >= 0x30) && (i <= 0x37)))
> + continue;
> + WREG_DAC(i, dacvalue[i]);
> + }
> +
> + mgag200_init_registers(mdev);
> +}
> +
> /*
> * DRM Device
> */
> @@ -191,6 +226,8 @@ struct mga_device *mgag200_g200_device_create(struct pci_dev *pdev, const struct
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200_init_registers(g200);
> +
> vram_available = mgag200_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh.c b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
> index 1b9a22728744..365f486d3ce7 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200eh.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
> @@ -6,6 +6,40 @@
>
> #include "mgag200_drv.h"
>
> +void mgag200_g200eh_init_registers(struct mga_device *mdev)
> +{
> + static const u8 dacvalue[] = {
> + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20,
> + /* 0x1e: */ MGA1064_MISC_CTL_VGA8 | /* G200EH specific: MGA1064_MISC_CTL */
> + MGA1064_MISC_CTL_DAC_RAM_CS,
> + /* 0x1f: */ 0x20,
> + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40,
> + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0
> + };
> +
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
> + if ((i <= 0x17) ||
> + (i == 0x1b) ||
> + (i == 0x1c) ||
> + ((i >= 0x1f) && (i <= 0x29)) ||
> + ((i >= 0x30) && (i <= 0x37)) ||
> + ((i >= 0x44) && (i <= 0x4e)))
> + continue;
> + WREG_DAC(i, dacvalue[i]);
> + }
> +
> + mgag200_init_registers(mdev);
> +}
> +
> /*
> * DRM device
> */
> @@ -40,6 +74,8 @@ struct mga_device *mgag200_g200eh_device_create(struct pci_dev *pdev, const stru
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200eh_init_registers(mdev);
> +
> vram_available = mgag200_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
> index 438cda1b14c9..adb9190b62af 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
> @@ -41,6 +41,8 @@ struct mga_device *mgag200_g200eh3_device_create(struct pci_dev *pdev,
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200eh_init_registers(mdev); // same as G200EH
> +
> vram_available = mgag200_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> index 0790d4e6463d..acdccc419944 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
> @@ -6,6 +6,38 @@
>
> #include "mgag200_drv.h"
>
> +static void mgag200_g200er_init_registers(struct mga_device *mdev)
> +{
> + static const u8 dacvalue[] = {
> + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
> + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40,
> + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0
> + };
> +
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
> + if ((i <= 0x17) ||
> + (i == 0x1b) ||
> + (i == 0x1c) ||
> + ((i >= 0x1f) && (i <= 0x29)) ||
> + ((i >= 0x30) && (i <= 0x37)))
> + continue;
> + WREG_DAC(i, dacvalue[i]);
> + }
> +
> + WREG_DAC(0x90, 0); /* G200ER specific */
> +
> + mgag200_init_registers(mdev);
> +}
> +
> /*
> * DRM device
> */
> @@ -36,6 +68,8 @@ struct mga_device *mgag200_g200er_device_create(struct pci_dev *pdev, const stru
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200er_init_registers(mdev);
> +
> vram_available = mgag200_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> index 5353422d0eef..ce26d48c9691 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
> @@ -6,6 +6,44 @@
>
> #include "mgag200_drv.h"
>
> +static void mgag200_g200ev_init_registers(struct mga_device *mdev)
> +{
> + static const u8 dacvalue[] = {
> + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x18: */ 0x00, 0,
> + /* G200EV specific: MGA1064_PIX_CLK_CTL */
> + /* 0x1a: */ MGA1064_PIX_CLK_CTL_SEL_PLL,
> + /* 0x1b: */ 0xFF, 0xBF, 0x20,
> + /* G200EV specific: MGA1064_MISC_CTL */
> + /* 0x1e: */ MGA1064_MISC_CTL_VGA8 |
> + MGA1064_MISC_CTL_DAC_RAM_CS,
> + /* 0x1f: */ 0x20,
> + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40,
> + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0
> + };
> +
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
> + if ((i <= 0x17) ||
> + (i == 0x1b) ||
> + (i == 0x1c) ||
> + ((i >= 0x1f) && (i <= 0x29)) ||
> + ((i >= 0x30) && (i <= 0x37)) ||
> + ((i >= 0x44) && (i <= 0x4e)))
> + continue;
> + WREG_DAC(i, dacvalue[i]);
> + }
> +
> + mgag200_init_registers(mdev);
> +}
> +
> /*
> * DRM device
> */
> @@ -40,6 +78,8 @@ struct mga_device *mgag200_g200ev_device_create(struct pci_dev *pdev, const stru
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200ev_init_registers(mdev);
> +
> vram_available = mgag200_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
> index 3bfc1324cf78..d86284c0eb4d 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
> @@ -50,6 +50,8 @@ struct mga_device *mgag200_g200ew3_device_create(struct pci_dev *pdev,
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200wb_init_registers(mdev); // same as G200WB
> +
> vram_available = mgag200_g200ew3_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200se.c b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> index 0a3e66695e22..9f51be8ef731 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200se.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200se.c
> @@ -28,6 +28,49 @@ static int mgag200_g200se_init_pci_options(struct pci_dev *pdev)
> return mgag200_init_pci_options(pdev, option, 0x00008000);
> }
>
> +static void mgag200_g200se_init_registers(struct mgag200_g200se_device *g200se)
> +{
> + static const u8 dacvalue[] = {
> + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0,
> + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* G200SE specific: MGA1064_VREF_CTL */
> + /* 0x18: */ 0x03,
> + /* 0x19: */ 0,
> + /* G200EV specific: MGA1064_PIX_CLK_CTL */
> + /* 0x1a: */ MGA1064_PIX_CLK_CTL_SEL_PLL,
> + /* 0x1b: */ 0xFF, 0xBF, 0x20,
> + /* G200EV specific: MGA1064_MISC_CTL */
> + /* 0x1e: */ MGA1064_MISC_CTL_DAC_EN |
> + MGA1064_MISC_CTL_VGA8 |
> + MGA1064_MISC_CTL_DAC_RAM_CS,
> +
> + /* 0x19: */ 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
> + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40,
> + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0
> + };
> +
> + struct mga_device *mdev = &g200se->base;
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
> + if ((i <= 0x17) ||
> + (i == 0x1b) ||
> + (i == 0x1c) ||
> + ((i >= 0x1f) && (i <= 0x29)) ||
> + ((i == 0x2c) || (i == 0x2d) || (i == 0x2e)) ||
> + ((i >= 0x30) && (i <= 0x37)))
> + continue;
> + WREG_DAC(i, dacvalue[i]);
> + }
> +
> + mgag200_init_registers(mdev);
> +}
> +
> /*
> * DRM device
> */
> @@ -120,6 +163,8 @@ struct mga_device *mgag200_g200se_device_create(struct pci_dev *pdev, const stru
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200se_init_registers(g200se);
> +
> vram_available = mgag200_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_g200wb.c b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
> index c8450ac8eaec..e4de4b2dc2e2 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_g200wb.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
> @@ -6,6 +6,38 @@
>
> #include "mgag200_drv.h"
>
> +void mgag200_g200wb_init_registers(struct mga_device *mdev)
> +{
> + static const u8 dacvalue[] = {
> + /* 0x00: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x18: */ 0x07, /* G200WB specific: MGA1064_VREF_CTL */
> + /* 0x19: */ 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
> + /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> + /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40,
> + /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> + /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> + /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0,
> + /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0
> + };
> +
> + size_t i;
> +
> + for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
> + if ((i <= 0x17) ||
> + (i == 0x1b) ||
> + (i == 0x1c) ||
> + ((i >= 0x1f) && (i <= 0x29)) ||
> + ((i >= 0x30) && (i <= 0x37)) ||
> + ((i >= 0x44) && (i <= 0x4e)))
> + continue;
> + WREG_DAC(i, dacvalue[i]);
> + }
> +
> + mgag200_init_registers(mdev);
> +}
> +
> /*
> * DRM device
> */
> @@ -40,6 +72,8 @@ struct mga_device *mgag200_g200wb_device_create(struct pci_dev *pdev, const stru
> if (ret)
> return ERR_PTR(ret);
>
> + mgag200_g200wb_init_registers(mdev);
> +
> vram_available = mgag200_device_probe_vram(mdev);
>
> ret = mgag200_modeset_init(mdev, vram_available);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index e08852482fe2..f9868d728e6d 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -267,86 +267,10 @@ static void mgag200_set_startadd(struct mga_device *mdev,
> WREG_ECRT(0x00, crtcext0);
> }
>
> -static void mgag200_set_dac_regs(struct mga_device *mdev)
> -{
> - size_t i;
> - u8 dacvalue[] = {
> - /* 0x00: */ 0, 0, 0, 0, 0, 0, 0x00, 0,
> - /* 0x08: */ 0, 0, 0, 0, 0, 0, 0, 0,
> - /* 0x10: */ 0, 0, 0, 0, 0, 0, 0, 0,
> - /* 0x18: */ 0x00, 0, 0xC9, 0xFF, 0xBF, 0x20, 0x1F, 0x20,
> - /* 0x20: */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> - /* 0x28: */ 0x00, 0x00, 0x00, 0x00, 0, 0, 0, 0x40,
> - /* 0x30: */ 0x00, 0xB0, 0x00, 0xC2, 0x34, 0x14, 0x02, 0x83,
> - /* 0x38: */ 0x00, 0x93, 0x00, 0x77, 0x00, 0x00, 0x00, 0x3A,
> - /* 0x40: */ 0, 0, 0, 0, 0, 0, 0, 0,
> - /* 0x48: */ 0, 0, 0, 0, 0, 0, 0, 0
> - };
> -
> - switch (mdev->type) {
> - case G200_PCI:
> - case G200_AGP:
> - dacvalue[MGA1064_SYS_PLL_M] = 0x04;
> - dacvalue[MGA1064_SYS_PLL_N] = 0x2D;
> - dacvalue[MGA1064_SYS_PLL_P] = 0x19;
> - break;
> - case G200_SE_A:
> - case G200_SE_B:
> - dacvalue[MGA1064_VREF_CTL] = 0x03;
> - dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
> - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_DAC_EN |
> - MGA1064_MISC_CTL_VGA8 |
> - MGA1064_MISC_CTL_DAC_RAM_CS;
> - break;
> - case G200_WB:
> - case G200_EW3:
> - dacvalue[MGA1064_VREF_CTL] = 0x07;
> - break;
> - case G200_EV:
> - dacvalue[MGA1064_PIX_CLK_CTL] = MGA1064_PIX_CLK_CTL_SEL_PLL;
> - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
> - MGA1064_MISC_CTL_DAC_RAM_CS;
> - break;
> - case G200_EH:
> - case G200_EH3:
> - dacvalue[MGA1064_MISC_CTL] = MGA1064_MISC_CTL_VGA8 |
> - MGA1064_MISC_CTL_DAC_RAM_CS;
> - break;
> - case G200_ER:
> - break;
> - }
> -
> - for (i = 0; i < ARRAY_SIZE(dacvalue); i++) {
> - if ((i <= 0x17) ||
> - (i == 0x1b) ||
> - (i == 0x1c) ||
> - ((i >= 0x1f) && (i <= 0x29)) ||
> - ((i >= 0x30) && (i <= 0x37)))
> - continue;
> - if (IS_G200_SE(mdev) &&
> - ((i == 0x2c) || (i == 0x2d) || (i == 0x2e)))
> - continue;
> - if ((mdev->type == G200_EV ||
> - mdev->type == G200_WB ||
> - mdev->type == G200_EH ||
> - mdev->type == G200_EW3 ||
> - mdev->type == G200_EH3) &&
> - (i >= 0x44) && (i <= 0x4e))
> - continue;
> -
> - WREG_DAC(i, dacvalue[i]);
> - }
> -
> - if (mdev->type == G200_ER)
> - WREG_DAC(0x90, 0);
> -}
> -
> -static void mgag200_init_regs(struct mga_device *mdev)
> +void mgag200_init_registers(struct mga_device *mdev)
> {
> u8 crtc11, misc;
>
> - mgag200_set_dac_regs(mdev);
> -
> WREG_SEQ(2, 0x0f);
> WREG_SEQ(3, 0x00);
> WREG_SEQ(4, 0x0e);
> @@ -1127,8 +1051,6 @@ int mgag200_modeset_init(struct mga_device *mdev, resource_size_t vram_available
> struct drm_device *dev = &mdev->base;
> int ret;
>
> - mgag200_init_regs(mdev);
> -
> ret = mgag200_mode_config_init(mdev, vram_available);
> if (ret)
> return ret;
> --
> 2.36.1
next prev parent reply other threads:[~2022-07-08 15:54 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-08 9:39 [PATCH 00/14] drm/mgag200: Move model-specific code into separate functions Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 01/14] drm/mgag200: Split mgag200_modeset_init() Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 02/14] drm/mgag200: Move DAC-register setup into model-specific code Thomas Zimmermann
2022-07-08 15:54 ` Sam Ravnborg [this message]
2022-07-11 7:25 ` Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 03/14] dmr/mgag200: Move ER/EW3 register initializatino to per-model code Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 04/14] drm/mgag200: Acquire I/O-register lock in atomic_commit_tail function Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 05/14] drm/mgag200: Store primary plane's color format in CRTC state Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 06/14] drm/mgag200: Reorganize before dropping simple-KMS helpers Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 07/14] drm/mgag200: Replace simple-KMS with regular atomic helpers Thomas Zimmermann
[not found] ` <20230425142519.13201-1-kernel@linuxace.com>
2023-04-25 15:03 ` Thomas Zimmermann
2023-04-25 15:48 ` Jocelyn Falempe
2023-04-25 19:56 ` kernel
2023-04-25 23:39 ` kernel
2023-04-26 7:01 ` Thomas Zimmermann
2023-04-26 10:15 ` Thomas Zimmermann
2023-04-27 0:47 ` kernel
2022-07-08 9:39 ` [PATCH 08/14] drm/mgag200: Set SCROFF in primary-plane code Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 09/14] drm/mgag200: Add per-device callbacks Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 10/14] drm/mgag200: Provide per-device callbacks for BMC synchronization Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 11/14] drm/mgag200: Provide per-device callbacks for PIXPLLC Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 12/14] drm/mgag200: Move mode-config to model-specific code Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 13/14] drm/mgag200: Move CRTC atomic_enable to model-specfic code Thomas Zimmermann
2022-07-08 9:39 ` [PATCH 14/14] drm/mgag200: Remove type field from struct mga_device Thomas Zimmermann
2022-07-08 17:18 ` Sam Ravnborg
2022-07-08 17:22 ` [PATCH 00/14] drm/mgag200: Move model-specific code into separate functions Sam Ravnborg
2022-07-11 7:29 ` Thomas Zimmermann
2022-07-11 16:20 ` Jocelyn Falempe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YshTN7eZFTdAka/E@ravnborg.org \
--to=sam@ravnborg.org \
--cc=airlied@linux.ie \
--cc=airlied@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=jfalempe@redhat.com \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.