On Fri, Oct 28, 2016 at 3:27 AM, Rongrong Zou <zourongrong@xxxxxxxxx> wrote:
> Add DRM master driver for Hisilicon Hibmc SoC which used for
> Out-of-band management. Blow is the general hardware connection,
> both the Hibmc and the host CPU are on the same mother board.
>
> +----------+ +----------+
> | | PCIe | Hibmc |
> |host CPU( |<----->| display |
> |arm64,x86)| |subsystem |
> +----------+ +----------+
>
> Signed-off-by: Rongrong Zou <zourongrong@xxxxxxxxx>
> ---
> drivers/gpu/drm/hisilicon/Kconfig | 1 +
> drivers/gpu/drm/hisilicon/Makefile | 1 +
> drivers/gpu/drm/hisilicon/hibmc/Kconfig | 7 +
> drivers/gpu/drm/hisilicon/hibmc/Makefile | 5 +
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 269 ++++++++++++++++++++++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 35 +++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c | 85 +++++++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h | 28 +++
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h | 212 +++++++++++++++++
> 9 files changed, 643 insertions(+)
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/Kconfig
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/Makefile
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h
> create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h
>
> diff --git a/drivers/gpu/drm/hisilicon/Kconfig b/drivers/gpu/drm/hisilicon/Kconfig
> index 558c61b..2fd2724 100644
> --- a/drivers/gpu/drm/hisilicon/Kconfig
> +++ b/drivers/gpu/drm/hisilicon/Kconfig
> @@ -2,4 +2,5 @@
> # hisilicon drm device configuration.
> # Please keep this list sorted alphabetically
>
> +source "drivers/gpu/drm/hisilicon/hibmc/Kconfig"
> source "drivers/gpu/drm/hisilicon/kirin/Kconfig"
> diff --git a/drivers/gpu/drm/hisilicon/Makefile b/drivers/gpu/drm/hisilicon/Makefile
> index e3f6d49..c8155bf 100644
> --- a/drivers/gpu/drm/hisilicon/Makefile
> +++ b/drivers/gpu/drm/hisilicon/Makefile
> @@ -2,4 +2,5 @@
> # Makefile for hisilicon drm drivers.
> # Please keep this list sorted alphabetically
>
> +obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc/
> obj-$(CONFIG_DRM_HISI_KIRIN) += kirin/
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Kconfig b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> new file mode 100644
> index 0000000..a9af90d
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Kconfig
> @@ -0,0 +1,7 @@
> +config DRM_HISI_HIBMC
> + tristate "DRM Support for Hisilicon Hibmc"
> + depends on DRM && PCI
> +
> + help
> + Choose this option if you have a Hisilicon Hibmc soc chipset.
> + If M is selected the module will be called hibmc-drm.
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> new file mode 100644
> index 0000000..97cf4a0
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -0,0 +1,5 @@
> +ccflags-y := -Iinclude/drm
> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_power.o
> +
> +obj-$(CONFIG_DRM_HISI_HIBMC) +=hibmc-drm.o
nit: Improper spacing here
> +#obj-y += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> new file mode 100644
> index 0000000..4669d42
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -0,0 +1,269 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Rongrong Zou <zourongrong@xxxxxxxxxx>
> + * Rongrong Zou <zourongrong@xxxxxxxxx>
> + * Jianhua Li <lijianhua@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/console.h>
> +
> +#include "hibmc_drm_drv.h"
> +#include "hibmc_drm_regs.h"
> +#include "hibmc_drm_power.h"
nit: Alphabetize headers
> +
> +static const struct file_operations hibmc_fops = {
> + .owner = THIS_MODULE,
> + .open = drm_open,
> + .release = drm_release,
> + .unlocked_ioctl = drm_ioctl,
> +#ifdef CONFIG_COMPAT
drm_compat_ioctl is now initialized to NULL, so you can remove the #ifdef
> + .compat_ioctl = drm_compat_ioctl,
> +#endif
> + .poll = drm_poll,
> + .read = drm_read,
> + .llseek = no_llseek,
> +};
> +
> +static int hibmc_enable_vblank(struct drm_device *dev, unsigned int pipe)
> +{
> + return 0;
> +}
> +
> +static void hibmc_disable_vblank(struct drm_device *dev, unsigned int pipe)
> +{
> +}
> +
> +static struct drm_driver hibmc_driver = {
> + .fops = &hibmc_fops,
> + .name = "hibmc",
> + .date = "20160828",
> + .desc = "hibmc drm driver",
> + .major = 1,
> + .minor = 0,
> + .get_vblank_counter = drm_vblank_no_hw_counter,
> + .enable_vblank = hibmc_enable_vblank,
> + .disable_vblank = hibmc_disable_vblank,
> +};
> +
> +static int hibmc_pm_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int hibmc_pm_resume(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static const struct dev_pm_ops hibmc_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(hibmc_pm_suspend,
> + hibmc_pm_resume)
> +};
> +
> +static int hibmc_hw_config(struct hibmc_drm_device *hidev)
> +{
> + unsigned int reg;
> +
> + /* On hardware reset, power mode 0 is default. */
> + hibmc_set_power_mode(hidev, HIBMC_PW_MODE_CTL_MODE_MODE0);
> +
> + /* Enable display power gate & LOCALMEM power gate*/
> + reg = readl(hidev->mmio + HIBMC_CURRENT_GATE);
> + reg &= ~HIBMC_CURR_GATE_DISPLAY_MASK;
> + reg &= ~HIBMC_CURR_GATE_LOCALMEM_MASK;
> + reg |= HIBMC_CURR_GATE_DISPLAY(ON);
> + reg |= HIBMC_CURR_GATE_LOCALMEM(ON);
> +
> + hibmc_set_current_gate(hidev, reg);
> +
> + /* Reset the memory controller. If the memory controller
> + * is not reset in chip,the system might hang when sw accesses
> + * the memory.The memory should be resetted after
> + * changing the MXCLK.
> + */
> + reg = readl(hidev->mmio + HIBMC_MISC_CTRL);
> + reg &= ~HIBMC_MSCCTL_LOCALMEM_RESET_MASK;
> + reg |= HIBMC_MSCCTL_LOCALMEM_RESET(RESET);
> + writel(reg, hidev->mmio + HIBMC_MISC_CTRL);
> +
> + reg &= ~HIBMC_MSCCTL_LOCALMEM_RESET_MASK;
> + reg |= HIBMC_MSCCTL_LOCALMEM_RESET(NORMAL);
> +
> + writel(reg, hidev->mmio + HIBMC_MISC_CTRL);
> +
> + /* We can add more initialization as needed. */
> +
> + return 0;
Consider using void return type here to simplify error checking in the
caller, especially since it looks like you aren't checking the return
code, anyways :)
> +}
> +
> +static int hibmc_hw_map(struct hibmc_drm_device *hidev)
> +{
> + struct drm_device *dev = hidev->dev;
> + struct pci_dev *pdev = dev->pdev;
> + resource_size_t addr, size, ioaddr, iosize;
> +
> + ioaddr = pci_resource_start(pdev, 1);
> + iosize = MB(2);
> +
> + hidev->mmio = ioremap_nocache(ioaddr, iosize);
Use devm_ioremap_nocache to avoid managing the resource directly
> +
nit: extra space
> + if (!hidev->mmio) {
> + DRM_ERROR("Cannot map mmio region\n");
> + return -ENOMEM;
> + }
> +
> + addr = pci_resource_start(pdev, 0);
> + size = MB(32);
Pull size and iosize out into #defines with descriptive names
> +
> + hidev->fb_map = ioremap(addr, size);
Use devm_ioremap to avoid managing the resource directly.
> + if (!hidev->fb_map) {
> + DRM_ERROR("Cannot map framebuffer\n");
> + return -ENOMEM;
> + }
> + hidev->fb_base = addr;
> + hidev->fb_size = size;
> +
> + return 0;
> +}
> +
> +static void hibmc_hw_fini(struct hibmc_drm_device *hidev)
> +{
> + if (hidev->mmio)
> + iounmap(hidev->mmio);
> + if (hidev->fb_map)
> + iounmap(hidev->fb_map);
> +}
You don't need this function if you use the devm variants above
> +
> +static int hibmc_hw_init(struct hibmc_drm_device *hidev)
> +{
> + int ret;
> +
> + ret = hibmc_hw_map(hidev);
> + if (ret)
> + return ret;
> +
> + hibmc_hw_config(hidev);
> +
> + return 0;
> +}
> +
> +static int hibmc_unload(struct drm_device *dev)
> +{
> + struct hibmc_drm_device *hidev = dev->dev_private;
> +
> + hibmc_hw_fini(hidev);
> + dev->dev_private = NULL;
> + return 0;
> +}
> +
> +static int hibmc_load(struct drm_device *dev, unsigned long flags)
flags isn't used anywhere?
> +{
> + struct hibmc_drm_device *hidev;
> + int ret;
> +
> + hidev = devm_kzalloc(dev->dev, sizeof(*hidev), GFP_KERNEL);
> + if (!hidev)
Print error here?
> + return -ENOMEM;
> + dev->dev_private = hidev;
> + hidev->dev = dev;
> +
> + ret = hibmc_hw_init(hidev);
> + if (ret)
> + goto err;
> +
> + return 0;
> +
> +err:
> + hibmc_unload(dev);
> + DRM_ERROR("failed to initialize drm driver.\n");
Print the return value
> + return ret;
> +}
> +
> +static int hibmc_pci_probe(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + struct drm_device *dev;
> + int ret;
> +
> + dev = drm_dev_alloc(&hibmc_driver, &pdev->dev);
> + if (!dev)
Print error here
> + return -ENOMEM;
> +
> + dev->pdev = pdev;
> + pci_set_drvdata(pdev, dev);
> +
> + ret = pci_enable_device(pdev);
> + if (ret)
and here, and in other failure paths
> + goto err_free;
> +
> + ret = hibmc_load(dev, 0);
> + if (ret)
> + goto err_disable;
> +
> + ret = drm_dev_register(dev, 0);
> + if (ret)
> + goto err_unload;
> +
> + return 0;
> +
> +err_unload:
> + hibmc_unload(dev);
> +err_disable:
> + pci_disable_device(pdev);
> +err_free:
> + drm_dev_unref(dev);
> +
> + return ret;
> +}
> +
> +static void hibmc_pci_remove(struct pci_dev *pdev)
> +{
> + struct drm_device *dev = pci_get_drvdata(pdev);
> +
> + drm_dev_unregister(dev);
> + hibmc_unload(dev);
> + drm_dev_unref(dev);
> +}
> +
> +static struct pci_device_id hibmc_pci_table[] = {
> + {0x19e5, 0x1711, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> + {0,}
> +};
> +
> +static struct pci_driver hibmc_pci_driver = {
> + .name = "hibmc-drm",
> + .id_table = hibmc_pci_table,
> + .probe = hibmc_pci_probe,
> + .remove = hibmc_pci_remove,
> + .driver.pm = &hibmc_pm_ops,
> +};
> +
> +static int __init hibmc_init(void)
> +{
> + return pci_register_driver(&hibmc_pci_driver);
> +}
> +
> +static void __exit hibmc_exit(void)
> +{
> + return pci_unregister_driver(&hibmc_pci_driver);
> +}
> +
> +module_init(hibmc_init);
> +module_exit(hibmc_exit);
> +
> +MODULE_DEVICE_TABLE(pci, hibmc_pci_table);
> +MODULE_AUTHOR("RongrongZou <zourongrong@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("DRM Driver for Hisilicon Hibmc");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> new file mode 100644
> index 0000000..0037341
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -0,0 +1,35 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Rongrong Zou <zourongrong@xxxxxxxxxx>
> + * Rongrong Zou <zourongrong@xxxxxxxxx>
> + * Jianhua Li <lijianhua@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#ifndef HIBMC_DRM_DRV_H
> +#define HIBMC_DRM_DRV_H
> +
> +#include <drm/drmP.h>
> +
> +struct hibmc_drm_device {
nit: Calling this hibmc_drm_priv would probably make things easier to
read. When I read hibmc_drm_device, it makes me think that it's an
extension of drm_device, which this isn't.
> + /* hw */
> + void __iomem *mmio;
> + void __iomem *fb_map;
> + unsigned long fb_base;
> + unsigned long fb_size;
> +
> + /* drm */
> + struct drm_device *dev;
> +};
> +
> +#endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c
> new file mode 100644
> index 0000000..1036542
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.c
I don't think you need a new file for these functions. Just stash them
in hibmc_drm_drv.c
> @@ -0,0 +1,85 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Rongrong Zou <zourongrong@xxxxxxxxxx>
> + * Rongrong Zou <zourongrong@xxxxxxxxx>
> + * Jianhua Li <lijianhua@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include "hibmc_drm_drv.h"
> +#include "hibmc_drm_regs.h"
> +
> +/*
> + * It can operate in one of three modes: 0, 1 or Sleep.
> + */
> +void hibmc_set_power_mode(struct hibmc_drm_device *hidev,
> + unsigned int power_mode)
> +{
> + unsigned int control_value = 0;
> + void __iomem *mmio = hidev->mmio;
> +
> + if (power_mode > HIBMC_PW_MODE_CTL_MODE_SLEEP)
> + return;
> +
> + control_value = readl(mmio + HIBMC_POWER_MODE_CTRL);
> + control_value &= ~HIBMC_PW_MODE_CTL_MODE_MASK;
> +
> + control_value |= HIBMC_PW_MODE_CTL_MODE(power_mode) &
> + HIBMC_PW_MODE_CTL_MODE_MASK;
> +
> + /* Set up other fields in Power Control Register */
> + if (power_mode == HIBMC_PW_MODE_CTL_MODE_SLEEP) {
> + control_value &= ~HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
You do this in both branches of the conditional
> + control_value |= HIBMC_PW_MODE_CTL_OSC_INPUT(0) &
> + HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
> + } else {
> + control_value &= ~HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
> + control_value |= HIBMC_PW_MODE_CTL_OSC_INPUT(1) &
> + HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
> + }
I think you could simplify this by adding a new local.
unsigned int input = 1;
if (power_mode == HIBMC_PW_MODE_CTL_MODE_SLEEP)
input = 0;
control_value = readl(mmio + HIBMC_POWER_MODE_CTRL);
control_value &= ~(HIBMC_PW_MODE_CTL_MODE_MASK |
HIBMC_PW_MODE_CTL_OSC_INPUT_MASK);
control_value |= HIBMC_PW_MODE_CTL_MODE(power_mode) &
HIBMC_PW_MODE_CTL_MODE_MASK;
control_value |= HIBMC_PW_MODE_CTL_OSC_INPUT(input) &
HIBMC_PW_MODE_CTL_OSC_INPUT_MASK;
> + /* Program new power mode. */
> + writel(control_value, mmio + HIBMC_POWER_MODE_CTRL);
> +}
> +
> +static unsigned int hibmc_get_power_mode(struct hibmc_drm_device *hidev)
> +{
> + void __iomem *mmio = hidev->mmio;
> +
> + return (readl(mmio + HIBMC_POWER_MODE_CTRL) &
> + HIBMC_PW_MODE_CTL_MODE_MASK) >> HIBMC_PW_MODE_CTL_MODE_SHIFT;
nit: You're doing a lot of work in the return statement here.
> +}
> +
> +void hibmc_set_current_gate(struct hibmc_drm_device *hidev, unsigned int gate)
> +{
> + unsigned int gate_reg;
> + unsigned int mode;
> + void __iomem *mmio = hidev->mmio;
> +
> + /* Get current power mode. */
nit: try to avoid comments that don't add value
> + mode = hibmc_get_power_mode(hidev);
> +
> + switch (mode) {
> + case HIBMC_PW_MODE_CTL_MODE_MODE0:
> + gate_reg = HIBMC_MODE0_GATE;
> + break;
> +
> + case HIBMC_PW_MODE_CTL_MODE_MODE1:
> + gate_reg = HIBMC_MODE1_GATE;
> + break;
> +
> + default:
> + gate_reg = HIBMC_MODE0_GATE;
> + break;
> + }
> + writel(gate, mmio + gate_reg);
> +}
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h
> new file mode 100644
> index 0000000..e20e1aa
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_power.h
> @@ -0,0 +1,28 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Rongrong Zou <zourongrong@xxxxxxxxxx>
> + * Rongrong Zou <zourongrong@xxxxxxxxx>
> + * Jianhua Li <lijianhua@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#ifndef HIBMC_DRM_POWER_H
> +#define HIBMC_DRM_POWER_H
> +
> +#include "hibmc_drm_drv.h"
> +
> +void hibmc_set_power_mode(struct hibmc_drm_device *hidev,
> + unsigned int power_mode);
> +void hibmc_set_current_gate(struct hibmc_drm_device *hidev,
> + unsigned int gate);
> +#endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h
> new file mode 100644
> index 0000000..9c804ca
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h
> @@ -0,0 +1,212 @@
> +/* Hisilicon Hibmc SoC drm driver
> + *
> + * Based on the bochs drm driver.
> + *
> + * Copyright (c) 2016 Huawei Limited.
> + *
> + * Author:
> + * Rongrong Zou <zourongrong@xxxxxxxxxx>
> + * Rongrong Zou <zourongrong@xxxxxxxxx>
> + * Jianhua Li <lijianhua@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#ifndef HIBMC_DRM_HW_H
> +#define HIBMC_DRM_HW_H
> +
> +#define OFF 0
> +#define ON 1
> +#define DISABLE 0
> +#define ENABLE 1
These are not good names. I think you can probably hardcode the 0's
and 1's in the code instead of these. However, if you want to use
them, at least add a HIBMC_ prefix
> +
> +/* register definition */
> +#define HIBMC_MISC_CTRL 0x4
> +
> +#define HIBMC_MSCCTL_LOCALMEM_RESET(x) ((x) << 6)
> +#define HIBMC_MSCCTL_LOCALMEM_RESET_MASK 0x40
> +
> +#define RESET 0
> +#define NORMAL 1
Same naming nit here. Add a prefix
> +
> +#define HIBMC_CURRENT_GATE 0x000040
> +#define HIBMC_CURR_GATE_DISPLAY(x) ((x) << 2)
> +#define HIBMC_CURR_GATE_DISPLAY_MASK 0x4
> +
> +#define HIBMC_CURR_GATE_LOCALMEM(x) ((x) << 1)
> +#define HIBMC_CURR_GATE_LOCALMEM_MASK 0x2
> +
> +#define HIBMC_MODE0_GATE 0x000044
> +#define HIBMC_MODE1_GATE 0x000048
> +#define HIBMC_POWER_MODE_CTRL 0x00004C
> +
> +#define HIBMC_PW_MODE_CTL_OSC_INPUT(x) ((x) << 3)
> +#define HIBMC_PW_MODE_CTL_OSC_INPUT_MASK 0x8
> +
> +#define HIBMC_PW_MODE_CTL_MODE(x) ((x) << 0)
> +#define HIBMC_PW_MODE_CTL_MODE_MASK 0x03
> +#define HIBMC_PW_MODE_CTL_MODE_SHIFT 0
> +
> +#define HIBMC_PW_MODE_CTL_MODE_MODE0 0
> +#define HIBMC_PW_MODE_CTL_MODE_MODE1 1
> +#define HIBMC_PW_MODE_CTL_MODE_SLEEP 2
> +
> +#define HIBMC_PANEL_PLL_CTRL 0x00005C
> +#define HIBMC_CRT_PLL_CTRL 0x000060
> +
> +#define HIBMC_PLL_CTRL_BYPASS(x) ((x) << 18)
> +#define HIBMC_PLL_CTRL_BYPASS_MASK 0x40000
> +
> +#define HIBMC_PLL_CTRL_POWER(x) ((x) << 17)
> +#define HIBMC_PLL_CTRL_POWER_MASK 0x20000
> +
> +#define HIBMC_PLL_CTRL_INPUT(x) ((x) << 16)
> +#define HIBMC_PLL_CTRL_INPUT_MASK 0x10000
> +
> +#define OSC 0
Naming
> +#define TESTCLK 1
This doesn't seem to be used?
> +
> +#define HIBMC_PLL_CTRL_POD(x) ((x) << 14)
> +#define HIBMC_PLL_CTRL_POD_MASK 0xC000
> +
> +#define HIBMC_PLL_CTRL_OD(x) ((x) << 12)
> +#define HIBMC_PLL_CTRL_OD_MASK 0x3000
> +
> +#define HIBMC_PLL_CTRL_N(x) ((x) << 8)
> +#define HIBMC_PLL_CTRL_N_MASK 0xF00
> +
> +#define HIBMC_PLL_CTRL_M(x) ((x) << 0)
> +#define HIBMC_PLL_CTRL_M_MASK 0xFF
> +
> +#define HIBMC_CRT_DISP_CTL 0x80200
> +
> +#define HIBMC_CRT_DISP_CTL_CRTSELECT(x) ((x) << 25)
> +#define HIBMC_CRT_DISP_CTL_CRTSELECT_MASK 0x2000000
> +
> +#define CRTSELECT_VGA 0
> +#define CRTSELECT_CRT 1
> +
> +#define HIBMC_CRT_DISP_CTL_CLOCK_PHASE(x) ((x) << 14)
> +#define HIBMC_CRT_DISP_CTL_CLOCK_PHASE_MASK 0x4000
> +
> +#define PHASE_ACTIVE_HIGH 0
> +#define PHASE_ACTIVE_LOW 1
> +
> +#define HIBMC_CRT_DISP_CTL_VSYNC_PHASE(x) ((x) << 13)
> +#define HIBMC_CRT_DISP_CTL_VSYNC_PHASE_MASK 0x2000
> +
> +#define HIBMC_CRT_DISP_CTL_HSYNC_PHASE(x) ((x) << 12)
> +#define HIBMC_CRT_DISP_CTL_HSYNC_PHASE_MASK 0x1000
> +
> +#define HIBMC_CRT_DISP_CTL_TIMING(x) ((x) << 8)
> +#define HIBMC_CRT_DISP_CTL_TIMING_MASK 0x100
> +
> +#define HIBMC_CRT_DISP_CTL_PLANE(x) ((x) << 2)
> +#define HIBMC_CRT_DISP_CTL_PLANE_MASK 4
> +
> +#define HIBMC_CRT_DISP_CTL_FORMAT(x) ((x) << 0)
> +#define HIBMC_CRT_DISP_CTL_FORMAT_MASK 0x03
> +
> +#define HIBMC_CRT_FB_ADDRESS 0x080204
> +
> +#define HIBMC_CRT_FB_WIDTH 0x080208
> +#define HIBMC_CRT_FB_WIDTH_WIDTH(x) ((x) << 16)
> +#define HIBMC_CRT_FB_WIDTH_WIDTH_MASK 0x3FFF0000
> +#define HIBMC_CRT_FB_WIDTH_OFFS(x) ((x) << 0)
> +#define HIBMC_CRT_FB_WIDTH_OFFS_MASK 0x3FFF
> +
> +#define HIBMC_CRT_HORZ_TOTAL 0x08020C
> +#define HIBMC_CRT_HORZ_TOTAL_TOTAL(x) ((x) << 16)
> +#define HIBMC_CRT_HORZ_TOTAL_TOTAL_MASK 0xFFF0000
> +
> +#define HIBMC_CRT_HORZ_TOTAL_DISPLAY_END(x) ((x) << 0)
> +#define HIBMC_CRT_HORZ_TOTAL_DISPLAY_END_MASK 0xFFF
> +
> +#define HIBMC_CRT_HORZ_SYNC 0x080210
> +#define HIBMC_CRT_HORZ_SYNC_WIDTH(x) ((x) << 16)
> +#define HIBMC_CRT_HORZ_SYNC_WIDTH_MASK 0xFF0000
> +
> +#define HIBMC_CRT_HORZ_SYNC_START(x) ((x) << 0)
> +#define HIBMC_CRT_HORZ_SYNC_START_MASK 0xFFF
> +
> +#define HIBMC_CRT_VERT_TOTAL 0x080214
> +#define HIBMC_CRT_VERT_TOTAL_TOTAL(x) ((x) << 16)
> +#define HIBMC_CRT_VERT_TOTAL_TOTAL_MASK 0x7FFF0000
> +
> +#define HIBMC_CRT_VERT_TOTAL_DISPLAY_END(x) ((x) << 0)
> +#define HIBMC_CRT_VERT_TOTAL_DISPLAY_END_MASK 0x7FF
> +
> +#define HIBMC_CRT_VERT_SYNC 0x080218
> +#define HIBMC_CRT_VERT_SYNC_HEIGHT(x) ((x) << 16)
> +#define HIBMC_CRT_VERT_SYNC_HEIGHT_MASK 0x3F0000
> +
> +#define HIBMC_CRT_VERT_SYNC_START(x) ((x) << 0)
> +#define HIBMC_CRT_VERT_SYNC_START_MASK 0x7FF
> +
> +/* Auto Centering */
> +#define HIBMC_CRT_AUTO_CENTERING_TL 0x080280
> +#define HIBMC_CRT_AUTO_CENTERING_TL_TOP(x) ((x) << 16)
> +#define HIBMC_CRT_AUTO_CENTERING_TL_TOP_MSK 0x7FF0000
> +
> +#define HIBMC_CRT_AUTO_CENTERING_TL_LEFT(x) ((x) << 0)
> +#define HIBMC_CRT_AUTO_CENTERING_TL_LEFT_MSK 0x7FF
> +
> +#define HIBMC_CRT_AUTO_CENTERING_BR 0x080284
> +#define HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM(x) ((x) << 16)
> +#define HIBMC_CRT_AUTO_CENTERING_BR_BOTTOM_MASK 0x7FF0000
> +
> +#define HIBMC_CRT_AUTO_CENTERING_BR_RIGHT(x) ((x) << 0)
> +#define HIBMC_CRT_AUTO_CENTERING_BR_RIGHT_MASK 0x7FF
> +
> +/* register to control panel output */
> +#define DISPLAY_CONTROL_HISILE 0x80288
> +
> +#define HIBMC_RAW_INTERRUPT 0x80290
> +#define HIBMC_RAW_INTERRUPT_VBLANK(x) ((x) << 2)
> +#define HIBMC_RAW_INTERRUPT_VBLANK_MASK 0x4
> +
> +#define HIBMC_RAW_INTERRUPT_EN 0x80298
> +#define HIBMC_RAW_INTERRUPT_EN_VBLANK(x) ((x) << 2)
> +#define HIBMC_RAW_INTERRUPT_EN_VBLANK_MASK 0x4
> +
> +/* register and values for PLL control */
> +#define CRT_PLL1_HS 0x802a8
> +#define CRT_PLL1_HS_25MHZ 0x23d40f02
> +#define CRT_PLL1_HS_40MHZ 0x23940801
> +#define CRT_PLL1_HS_65MHZ 0x23940d01
> +#define CRT_PLL1_HS_78MHZ 0x23540F82
> +#define CRT_PLL1_HS_74MHZ 0x23941dc2
> +#define CRT_PLL1_HS_80MHZ 0x23941001
> +#define CRT_PLL1_HS_80MHZ_1152 0x23540fc2
> +#define CRT_PLL1_HS_108MHZ 0x23b41b01
> +#define CRT_PLL1_HS_162MHZ 0x23480681
> +#define CRT_PLL1_HS_148MHZ 0x23541dc2
> +#define CRT_PLL1_HS_193MHZ 0x234807c1
> +
> +#define CRT_PLL2_HS 0x802ac
> +#define CRT_PLL2_HS_25MHZ 0x206B851E
> +#define CRT_PLL2_HS_40MHZ 0x30000000
> +#define CRT_PLL2_HS_65MHZ 0x40000000
> +#define CRT_PLL2_HS_78MHZ 0x50E147AE
> +#define CRT_PLL2_HS_74MHZ 0x602B6AE7
> +#define CRT_PLL2_HS_80MHZ 0x70000000
> +#define CRT_PLL2_HS_108MHZ 0x80000000
> +#define CRT_PLL2_HS_162MHZ 0xA0000000
> +#define CRT_PLL2_HS_148MHZ 0xB0CCCCCD
> +#define CRT_PLL2_HS_193MHZ 0xC0872B02
> +
> +/* Global macros */
> +#define RGB(r, g, b) \
Not used anywhere?
> +( \
> + (unsigned long)(((r) << 16) | ((g) << 8) | (b)) \
> +)
> +
> +#define PADDING(align, data) (((data) + (align) - 1) & (~((align) - 1)))
> +
This is only used in hibmc_drm_de.c, move it in there. It might also
be nice to make it a type-checked function while you're at it.
> +#define MB(x) ((x) << 20)
> +
This is only used in 2 places, I think you can just hardcode the value
in their respective #defines
> +#endif
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
