| Message ID | 20220321162916.1116541-1-15330273260@189.cn (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/lsdc: add drm driver for loongson display controller | expand |
On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote: > From: suijingfeng <suijingfeng@loongson.cn> > > There is a display controller in loongson's LS2K1000 SoC and LS7A1000 > bridge chip, the display controller is a PCI device in those chips. It > has two display pipes but with only one hardware cursor. Each way has > a DVO interface which provide RGB888 signals, vertical & horizontal > synchronisations, data enable and the pixel clock. Each CRTC is able to > scanout from 1920x1080 resolution at 60Hz, the maxmium resolution is > 2048x2048 according to the hardware spec. Loongson display controllers > are simple which require scanout buffers to be physically contiguous. > > For LS7A1000 bridge chip, the DC is equipped with a dedicated video RAM > which is typically 64MB or more. In this case, VRAM helper based driver > is intend to be used. While LS2K1000 is a SoC, only system memory is > available. Therefore CMA helper based driver is intend to be used. It is > possible to use VRAM helper based solution by carving out part of system > memory as VRAM though. > > For LS7A1000, there are 4 dedicated GPIOs whose control register is > located at the DC register space, They are used to emulate two way i2c. > One for DVO0, another for DVO1. LS2K1000 and LS2K0500 SoC don't have such > GPIO hardwared, they grab i2c adapter from other module, either general > purpose GPIO emulated i2c or hardware i2c adapter. > > +------+ +-----------------------------------+ > | DDR4 | | +-------------------+ | > +------+ | | PCIe Root complex | LS7A1000 | > || MC0 | +--++---------++----+ | > +----------+ HT 3.0 | || || | > | LS3A4000 |<-------->| +---++---+ +--++--+ +---------+ +------+ > | CPU |<-------->| | GC1000 | | LSDC |<-->| DDR3 MC |<->| VRAM | > +----------+ | +--------+ +-+--+-+ +---------+ +------+ > || MC1 +---------------|--|----------------+ > +------+ | | > | DDR4 | +-------+ DVO0 | | DVO1 +------+ > +------+ VGA <--|ADV7125|<--------+ +-------->|TFP410|--> DVI/HDMI > +-------+ +------+ > > The above picture give a simple usage of LS7A1000, note that the encoder > is not necessary adv7125 or tfp410, other candicates can be ch7034b, > sil9022, ite66121 and lt8618 etc. > > v2: Fixup warnings reported by kernel test robot > > v3: Fix more grammar mistakes in Kconfig reported by Randy Dunlap and give > more details about lsdc. > > v4: > 1) Add dts required and explain why device tree is required. > 2) Give more description about lsdc and VRAM helper based driver. > 3) Fix warnings reported by kernel test robot. > 4) Introduce stride_alignment member into struct lsdc_chip_desc, the > stride alignment is 256 bytes for ls7a1000, ls2k1000 and ls2k0500. > > v5: > 1) Using writel and readl replace writeq and readq, to fix kernel test > robot report build error on other archtecture. > 2) Set default fb format to XRGB8888 at crtc reset time. > > v6: > 1) Explain why we are not switch to drm dridge subsystem on ls2k1000. > 2) Explain why tiny drm driver is not suitable for us. > 3) Give a short description of the trival dirty uppdate implement based > on CMA helper. > > v7: > 1) Remove select I2C_GPIO and I2C_LS2X in Kconfig, it is not ready now > 2) Licensing issues are fixed suggested by Krzysztof Kozlowski. > 3) Remove lsdc_pixpll_print(), part of it move to debugfs. > 4) Set prefer_shadow to true if vram based driver is in using. > 5) Replace double blank lines with single line in all files. > 6) Verbose cmd line parameter is replaced with drm_dbg() > 7) All warnnings reported by ./scripts/checkpatch.pl --strict are fixed > 8) Get edid from dtb support is removed as suggested by Maxime Ripard > 9) Fix typos and various improvement > > v8: > 1) Drop damage update implement and its command line. > 2) Drop DRM_LSDC_VRAM_DRIVER config option as suggested by Maxime. > 3) Deduce DC's identification from its compatible property. > 4) Drop the board specific dts patch. > 5) Add documention about the display controller device node. > > v9: > 1) Fix the warnings reported by checkpatch script and fix typos > > v10: > 1) Pass `make dt_binding_check` validation > 2) Fix warnings reported by kernel test robot > > v11: > 1) Convert the driver to use drm bridge and of graph framework. > 2) Dump register value support through debugfs. > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: suijingfeng <suijingfeng@loongson.cn> > Signed-off-by: Sui Jingfeng <15330273260@189.cn> > Signed-off-by: suijingfeng <suijingfeng@loongson.cn> > --- > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/lsdc/Kconfig | 23 ++ > drivers/gpu/drm/lsdc/Makefile | 13 + > drivers/gpu/drm/lsdc/lsdc_crtc.c | 396 +++++++++++++++++++ > drivers/gpu/drm/lsdc/lsdc_drv.c | 547 ++++++++++++++++++++++++++ > drivers/gpu/drm/lsdc/lsdc_drv.h | 197 ++++++++++ > drivers/gpu/drm/lsdc/lsdc_i2c.c | 235 ++++++++++++ > drivers/gpu/drm/lsdc/lsdc_i2c.h | 42 ++ > drivers/gpu/drm/lsdc/lsdc_irq.c | 58 +++ > drivers/gpu/drm/lsdc/lsdc_irq.h | 18 + > drivers/gpu/drm/lsdc/lsdc_output.c | 262 +++++++++++++ > drivers/gpu/drm/lsdc/lsdc_output.h | 24 ++ > drivers/gpu/drm/lsdc/lsdc_pci_drv.c | 328 ++++++++++++++++ > drivers/gpu/drm/lsdc/lsdc_plane.c | 470 +++++++++++++++++++++++ > drivers/gpu/drm/lsdc/lsdc_pll.c | 574 ++++++++++++++++++++++++++++ > drivers/gpu/drm/lsdc/lsdc_pll.h | 88 +++++ > drivers/gpu/drm/lsdc/lsdc_regs.h | 220 +++++++++++ > 18 files changed, 3498 insertions(+) > create mode 100644 drivers/gpu/drm/lsdc/Kconfig > create mode 100644 drivers/gpu/drm/lsdc/Makefile > create mode 100644 drivers/gpu/drm/lsdc/lsdc_crtc.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.h > create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.h > create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.h > create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.h > create mode 100644 drivers/gpu/drm/lsdc/lsdc_pci_drv.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_plane.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.c > create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.h > create mode 100644 drivers/gpu/drm/lsdc/lsdc_regs.h [...] > diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.c b/drivers/gpu/drm/lsdc/lsdc_i2c.c > new file mode 100644 > index 000000000000..55beed9266fa > --- /dev/null > +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.c > @@ -0,0 +1,235 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * KMS driver for Loongson display controller Not really a useful comment since every file has the same one. > + * Copyright (C) 2022 Loongson Corporation > + */ > + > +/* > + * Authors: > + * Sui Jingfeng <suijingfeng@loongson.cn> > + */ > + > +#include <linux/i2c.h> > +#include <linux/pci.h> > + > +#include "lsdc_drv.h" > +#include "lsdc_regs.h" > +#include "lsdc_i2c.h" > + > +/* > + * ls7a_gpio_i2c_set - set the state of a gpio pin indicated by mask > + * @mask: gpio pin mask > + */ > +static void ls7a_gpio_i2c_set(struct lsdc_i2c * const li2c, int mask, int state) > +{ > + unsigned long flags; > + u8 val; > + > + spin_lock_irqsave(&li2c->reglock, flags); What are you protecting? Doesn't the caller serialize calls to these functions? > + > + if (state) { > + val = readb(li2c->dir_reg); > + val |= mask; > + writeb(val, li2c->dir_reg); > + } else { > + val = readb(li2c->dir_reg); > + val &= ~mask; > + writeb(val, li2c->dir_reg); > + > + val = readb(li2c->dat_reg); > + if (state) This condition is never true. We're in the 'else' because !state. > + val |= mask; > + else > + val &= ~mask; > + writeb(val, li2c->dat_reg); Shouldn't you set the data register low first and then change the direction? Otherwise, you may be driving high for a moment. However, if high is always done by setting the direction as input, why write the data register each time? I'm assuming whatever is written to the dat_reg is maintained regardless of pin state. > + } > + > + spin_unlock_irqrestore(&li2c->reglock, flags); > +} > + > +/* > + * ls7a_gpio_i2c_get - read value back from gpio pin > + * @mask: gpio pin mask > + */ > +static int ls7a_gpio_i2c_get(struct lsdc_i2c * const li2c, int mask) > +{ > + unsigned long flags; > + u8 val; > + > + spin_lock_irqsave(&li2c->reglock, flags); > + > + /* first set this pin as input */ > + val = readb(li2c->dir_reg); > + val |= mask; > + writeb(val, li2c->dir_reg); > + > + /* then get level state from this pin */ > + val = readb(li2c->dat_reg); > + > + spin_unlock_irqrestore(&li2c->reglock, flags); > + > + return (val & mask) ? 1 : 0; > +} > + > +/* set the state on the i2c->sda pin */ > +static void ls7a_i2c_set_sda(void *i2c, int state) > +{ > + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; > + > + return ls7a_gpio_i2c_set(li2c, li2c->sda, state); > +} > + > +/* set the state on the i2c->scl pin */ > +static void ls7a_i2c_set_scl(void *i2c, int state) > +{ > + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; > + > + return ls7a_gpio_i2c_set(li2c, li2c->scl, state); > +} > + > +/* read the value from the i2c->sda pin */ > +static int ls7a_i2c_get_sda(void *i2c) > +{ > + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; > + > + return ls7a_gpio_i2c_get(li2c, li2c->sda); > +} > + > +/* read the value from the i2c->scl pin */ > +static int ls7a_i2c_get_scl(void *i2c) > +{ > + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; > + > + return ls7a_gpio_i2c_get(li2c, li2c->scl); > +} > + > +/* > + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c > + * > + * @index : output channel index, 0 for DVO0, 1 for DVO1 > + */ > +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) > +{ > + char compat[32] = {0}; > + unsigned int udelay = 5; > + unsigned int timeout = 2200; > + int nr = -1; > + struct i2c_adapter *adapter; > + struct lsdc_i2c *li2c; > + struct device_node *i2c_np; > + int ret; > + > + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); > + if (!li2c) > + return ERR_PTR(-ENOMEM); > + > + li2c->index = index; > + li2c->dev = dev; > + > + if (index == 0) { > + li2c->sda = 0x01; > + li2c->scl = 0x02; > + } else if (index == 1) { > + li2c->sda = 0x04; > + li2c->scl = 0x08; Just require this to be in DT rather than having some default. > + } > + > + spin_lock_init(&li2c->reglock); > + > + snprintf(compat, sizeof(compat), "lsdc,i2c-gpio-%d", index); compatible values shouldn't have an index and you shouldn't need a index in DT. You need to iterate over child nodes with matching compatible. > + i2c_np = of_find_compatible_node(dev->of_node, NULL, compat); > + if (i2c_np) { > + u32 sda, scl; > + > + dev_dbg(dev, "Has %s property in the DT", compat); > + > + /* */ > + ret = of_property_read_u32(i2c_np, "sda", &sda); Custom properties need a vendor prefix. > + if (ret == 0) > + li2c->sda = 1 << sda; > + > + ret = of_property_read_u32(i2c_np, "scl", &scl); > + if (ret == 0) > + li2c->scl = 1 << scl; > + > + /* Optional properties which made the driver more flexible */ > + of_property_read_u32(i2c_np, "udelay", &udelay); > + of_property_read_u32(i2c_np, "timeout", &timeout); These aren't documented. Do you really need them in DT? > + of_property_read_u32(i2c_np, "reg", &nr); > + } > + > + dev_dbg(dev, "%s: sda=%u, scl=%u, nr=%d, udelay=%u, timeout=%u\n", > + compat, li2c->sda, li2c->scl, nr, udelay, timeout); > + > + li2c->reg_base = base; > + > + li2c->dir_reg = li2c->reg_base + LS7A_DC_GPIO_DIR_REG; > + li2c->dat_reg = li2c->reg_base + LS7A_DC_GPIO_DAT_REG; > + > + li2c->bit.setsda = ls7a_i2c_set_sda; > + li2c->bit.setscl = ls7a_i2c_set_scl; > + li2c->bit.getsda = ls7a_i2c_get_sda; > + li2c->bit.getscl = ls7a_i2c_get_scl; > + li2c->bit.udelay = udelay; > + li2c->bit.timeout = usecs_to_jiffies(timeout); > + li2c->bit.data = li2c; > + > + adapter = &li2c->adapter; > + adapter->algo_data = &li2c->bit; > + adapter->owner = THIS_MODULE; > + adapter->class = I2C_CLASS_DDC; > + adapter->dev.parent = dev; > + adapter->nr = nr; > + if (i2c_np) { > + adapter->dev.of_node = i2c_np; > + of_node_put(i2c_np); > + } > + > + strscpy(adapter->name, &compat[5], sizeof(adapter->name)); > + > + i2c_set_adapdata(adapter, li2c); > + > + ret = i2c_bit_add_numbered_bus(adapter); Why do you care what the bus number is? You shouldn't need to. > + if (ret) { > + if (i2c_np) > + of_node_put(i2c_np); > + > + devm_kfree(dev, li2c); > + return ERR_PTR(ret); > + } > + > + return li2c; > +} > + > +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c) > +{ > + struct i2c_adapter *adapter; > + > + if (li2c) { > + adapter = &li2c->adapter; > + > + if (adapter && adapter->dev.of_node) > + of_node_put(adapter->dev.of_node); > + > + devm_kfree(ddev->dev, li2c); > + } > +} > + > +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, > + unsigned int index) > +{ > + const struct lsdc_chip_desc * const descp = ldev->desc; > + struct lsdc_i2c *li2c; > + > + if (index >= descp->num_of_crtc) { > + drm_err(ldev->ddev, "I2c adapter is no more than %u, %u\n", > + descp->num_of_crtc, index); > + return NULL; > + } > + > + li2c = ldev->li2c[index]; > + if (li2c) > + return &li2c->adapter; > + > + return NULL; > +} > diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.h b/drivers/gpu/drm/lsdc/lsdc_i2c.h > new file mode 100644 > index 000000000000..4ab825143eb4 > --- /dev/null > +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.h > @@ -0,0 +1,42 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * KMS driver for Loongson display controller > + * Copyright (C) 2022 Loongson Corporation > + */ > + > +/* > + * Authors: > + * Sui Jingfeng <suijingfeng@loongson.cn> > + */ > + > +#ifndef __LSDC_I2C__ > +#define __LSDC_I2C__ > + > +#include <linux/i2c.h> > +#include <linux/i2c-algo-bit.h> > +#include <linux/pci.h> > + > +struct lsdc_i2c { > + struct device *dev; > + struct i2c_adapter adapter; > + struct i2c_algo_bit_data bit; > + /* @reglock: protects concurrent register access */ > + spinlock_t reglock; > + void __iomem *reg_base; > + void __iomem *dir_reg; > + void __iomem *dat_reg; > + int index; > + /* pin bit mask */ > + u8 sda; > + u8 scl; > +}; > + > +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c); > + > +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, > + void *base, > + unsigned int index); > + > +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, > + unsigned int index); > +#endif
On 2022/3/23 04:49, Rob Herring wrote: >> +/* >> + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c >> + * >> + * @index : output channel index, 0 for DVO0, 1 for DVO1 >> + */ >> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) >> +{ >> + char compat[32] = {0}; >> + unsigned int udelay = 5; >> + unsigned int timeout = 2200; >> + int nr = -1; >> + struct i2c_adapter *adapter; >> + struct lsdc_i2c *li2c; >> + struct device_node *i2c_np; >> + int ret; >> + >> + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); >> + if (!li2c) >> + return ERR_PTR(-ENOMEM); >> + >> + li2c->index = index; >> + li2c->dev = dev; >> + >> + if (index == 0) { >> + li2c->sda = 0x01; >> + li2c->scl = 0x02; >> + } else if (index == 1) { >> + li2c->sda = 0x04; >> + li2c->scl = 0x08; > Just require this to be in DT rather than having some default. > By design, I am try very hard to let the code NOT fully DT dependent. DT is nice , easy to learn and use. But kernel side developer plan to follow UEFI + ACPI Specification on LS3A5000 + LS7A1000 platform. See [1] There will no DT support then, provide a convention support make the driver more flexible. I want the driver works with minimal requirement. The driver just works on simple boards by put the following dc device node in arch/mips/dts/loongson/loongson64g_4core_ls7a.dts, lsdc: display-controller@6,1 { compatible = "loongson,ls7a1000-dc"; reg = <0x3100 0x0 0x0 0x0 0x0>; interrupts = <28 IRQ_TYPE_LEVEL_HIGH>; interrupt-parent = <&pic>; }; [1] https://lwn.net/Articles/869541/#:~:text=LoongArch%20is%20a%20new%20RISC%20ISA%2C%20which%20is,revision%20of%20ACPI%20Specification%20%28current%20revision%20is%206.4%29.
On 2022/3/23 04:49, Rob Herring wrote:
> This condition is never true. We're in the 'else' because !state.
Thanks for your sharp eyes, after the gpio emulate i2c driver works, i do not pay much
attention to it and get hurry to do other things. I will fix this issue at next version
and reply other problem at a letter time.
On 2022/3/23 04:49, Rob Herring wrote: >> +/* >> + * ls7a_gpio_i2c_set - set the state of a gpio pin indicated by mask >> + * @mask: gpio pin mask >> + */ >> +static void ls7a_gpio_i2c_set(struct lsdc_i2c * const li2c, int mask, int state) >> +{ >> + unsigned long flags; >> + u8 val; >> + >> + spin_lock_irqsave(&li2c->reglock, flags); > What are you protecting? Doesn't the caller serialize calls to these > functions? > This driver is ported from my work from my downstream work. Maxime also ask this question before, but i did not answer. He is right, protect single register access is not necessary. uncached access have done the job itself. so i remove it in V11 of my patch set. There are two way GPIO emulated i2c, I want the code between spin_lock_irqsave(&li2c->reglock, flags) and spin_unlock_irqrestore(&li2c->reglock, flags); finished in a atomic way, without any disruption. The two i2c should not have any influence each other. I write it by gut feeling, and luckily it works very well in practice.
On 2022/3/23 04:49, Rob Herring wrote: >> + >> + if (state) { >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + } else { >> + val = readb(li2c->dir_reg); >> + val &= ~mask; >> + writeb(val, li2c->dir_reg); >> + >> + val = readb(li2c->dat_reg); >> + if (state) > This condition is never true. We're in the 'else' because !state. > >> + val |= mask; >> + else >> + val &= ~mask; >> + writeb(val, li2c->dat_reg); > Shouldn't you set the data register low first and then change the > direction? Otherwise, you may be driving high for a moment. However, if > high is always done by setting the direction as input, why write the > data register each time? I'm assuming whatever is written to the dat_reg > is maintained regardless of pin state. To be honest, i have rewrite GPIO emulated i2c several times. Either give data first, then give the direction or give the direction first, then the data will be OK in practice. In the theory, the GPIO data should be given before the GPIO direction, I was told doing that way when learning Single-Chip Microcomputer (AT89S52). But the high "MUST" be done by setting the direction as input. It is "MUST" not "CAN" because writing code as the following way works in practice. if (state) { val = readb(li2c->dir_reg); val |= mask; writeb(val, li2c->dir_reg); } else { // ... } If the adjust the above code by first set the detection as output, then set the GPIO data register with high voltage level("1"). as the following demonstrate code, if (state) { /* First set this pin as output */ val = readb(li2c->dir_reg); val |= mask; writeb(val, li2c->dir_reg); /* Then, set the state to high */ val = readb(li2c->dat_reg); val |= mask; writeb(val, li2c->dat_reg); } else { // ... } Then i2c6 will NOT work as exacted, i2c7 will work, so strangely. It may because the GPIO is open drained, not Push-pull output. Output high is achieved by externalpull up resistance on the PCB.
On Wed, Mar 23, 2022 at 12:12:43PM +0800, Sui Jingfeng wrote: > > On 2022/3/23 04:49, Rob Herring wrote: > > > +/* > > > + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c > > > + * > > > + * @index : output channel index, 0 for DVO0, 1 for DVO1 > > > + */ > > > +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) > > > +{ > > > + char compat[32] = {0}; > > > + unsigned int udelay = 5; > > > + unsigned int timeout = 2200; > > > + int nr = -1; > > > + struct i2c_adapter *adapter; > > > + struct lsdc_i2c *li2c; > > > + struct device_node *i2c_np; > > > + int ret; > > > + > > > + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); > > > + if (!li2c) > > > + return ERR_PTR(-ENOMEM); > > > + > > > + li2c->index = index; > > > + li2c->dev = dev; > > > + > > > + if (index == 0) { > > > + li2c->sda = 0x01; > > > + li2c->scl = 0x02; > > > + } else if (index == 1) { > > > + li2c->sda = 0x04; > > > + li2c->scl = 0x08; > > Just require this to be in DT rather than having some default. > > > By design, I am try very hard to let the code NOT fully DT dependent. DT is nice , easy to learn and use. > But kernel side developer plan to follow UEFI + ACPI Specification on LS3A5000 + LS7A1000 platform. See [1] > There will no DT support then, provide a convention support make the driver more flexible. I want the > driver works with minimal requirement. The driver just works on simple boards by put the following dc device > node in arch/mips/dts/loongson/loongson64g_4core_ls7a.dts, Pick DT or ACPI for the platform, not both. We don't need to have both in the kernel to support. Rob
On 2022/3/23 04:49, Rob Herring wrote: > On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote: >> From: suijingfeng <suijingfeng@loongson.cn> >> >> There is a display controller in loongson's LS2K1000 SoC and LS7A1000 >> bridge chip, the display controller is a PCI device in those chips. It >> has two display pipes but with only one hardware cursor. Each way has >> a DVO interface which provide RGB888 signals, vertical & horizontal >> synchronisations, data enable and the pixel clock. Each CRTC is able to >> scanout from 1920x1080 resolution at 60Hz, the maxmium resolution is >> 2048x2048 according to the hardware spec. Loongson display controllers >> are simple which require scanout buffers to be physically contiguous. >> >> For LS7A1000 bridge chip, the DC is equipped with a dedicated video RAM >> which is typically 64MB or more. In this case, VRAM helper based driver >> is intend to be used. While LS2K1000 is a SoC, only system memory is >> available. Therefore CMA helper based driver is intend to be used. It is >> possible to use VRAM helper based solution by carving out part of system >> memory as VRAM though. >> >> For LS7A1000, there are 4 dedicated GPIOs whose control register is >> located at the DC register space, They are used to emulate two way i2c. >> One for DVO0, another for DVO1. LS2K1000 and LS2K0500 SoC don't have such >> GPIO hardwared, they grab i2c adapter from other module, either general >> purpose GPIO emulated i2c or hardware i2c adapter. >> >> +------+ +-----------------------------------+ >> | DDR4 | | +-------------------+ | >> +------+ | | PCIe Root complex | LS7A1000 | >> || MC0 | +--++---------++----+ | >> +----------+ HT 3.0 | || || | >> | LS3A4000 |<-------->| +---++---+ +--++--+ +---------+ +------+ >> | CPU |<-------->| | GC1000 | | LSDC |<-->| DDR3 MC |<->| VRAM | >> +----------+ | +--------+ +-+--+-+ +---------+ +------+ >> || MC1 +---------------|--|----------------+ >> +------+ | | >> | DDR4 | +-------+ DVO0 | | DVO1 +------+ >> +------+ VGA <--|ADV7125|<--------+ +-------->|TFP410|--> DVI/HDMI >> +-------+ +------+ >> >> The above picture give a simple usage of LS7A1000, note that the encoder >> is not necessary adv7125 or tfp410, other candicates can be ch7034b, >> sil9022, ite66121 and lt8618 etc. >> >> v2: Fixup warnings reported by kernel test robot >> >> v3: Fix more grammar mistakes in Kconfig reported by Randy Dunlap and give >> more details about lsdc. >> >> v4: >> 1) Add dts required and explain why device tree is required. >> 2) Give more description about lsdc and VRAM helper based driver. >> 3) Fix warnings reported by kernel test robot. >> 4) Introduce stride_alignment member into struct lsdc_chip_desc, the >> stride alignment is 256 bytes for ls7a1000, ls2k1000 and ls2k0500. >> >> v5: >> 1) Using writel and readl replace writeq and readq, to fix kernel test >> robot report build error on other archtecture. >> 2) Set default fb format to XRGB8888 at crtc reset time. >> >> v6: >> 1) Explain why we are not switch to drm dridge subsystem on ls2k1000. >> 2) Explain why tiny drm driver is not suitable for us. >> 3) Give a short description of the trival dirty uppdate implement based >> on CMA helper. >> >> v7: >> 1) Remove select I2C_GPIO and I2C_LS2X in Kconfig, it is not ready now >> 2) Licensing issues are fixed suggested by Krzysztof Kozlowski. >> 3) Remove lsdc_pixpll_print(), part of it move to debugfs. >> 4) Set prefer_shadow to true if vram based driver is in using. >> 5) Replace double blank lines with single line in all files. >> 6) Verbose cmd line parameter is replaced with drm_dbg() >> 7) All warnnings reported by ./scripts/checkpatch.pl --strict are fixed >> 8) Get edid from dtb support is removed as suggested by Maxime Ripard >> 9) Fix typos and various improvement >> >> v8: >> 1) Drop damage update implement and its command line. >> 2) Drop DRM_LSDC_VRAM_DRIVER config option as suggested by Maxime. >> 3) Deduce DC's identification from its compatible property. >> 4) Drop the board specific dts patch. >> 5) Add documention about the display controller device node. >> >> v9: >> 1) Fix the warnings reported by checkpatch script and fix typos >> >> v10: >> 1) Pass `make dt_binding_check` validation >> 2) Fix warnings reported by kernel test robot >> >> v11: >> 1) Convert the driver to use drm bridge and of graph framework. >> 2) Dump register value support through debugfs. >> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: suijingfeng <suijingfeng@loongson.cn> >> Signed-off-by: Sui Jingfeng <15330273260@189.cn> >> Signed-off-by: suijingfeng <suijingfeng@loongson.cn> >> --- >> drivers/gpu/drm/Kconfig | 2 + >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/lsdc/Kconfig | 23 ++ >> drivers/gpu/drm/lsdc/Makefile | 13 + >> drivers/gpu/drm/lsdc/lsdc_crtc.c | 396 +++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_drv.c | 547 ++++++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_drv.h | 197 ++++++++++ >> drivers/gpu/drm/lsdc/lsdc_i2c.c | 235 ++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_i2c.h | 42 ++ >> drivers/gpu/drm/lsdc/lsdc_irq.c | 58 +++ >> drivers/gpu/drm/lsdc/lsdc_irq.h | 18 + >> drivers/gpu/drm/lsdc/lsdc_output.c | 262 +++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_output.h | 24 ++ >> drivers/gpu/drm/lsdc/lsdc_pci_drv.c | 328 ++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_plane.c | 470 +++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_pll.c | 574 ++++++++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_pll.h | 88 +++++ >> drivers/gpu/drm/lsdc/lsdc_regs.h | 220 +++++++++++ >> 18 files changed, 3498 insertions(+) >> create mode 100644 drivers/gpu/drm/lsdc/Kconfig >> create mode 100644 drivers/gpu/drm/lsdc/Makefile >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_crtc.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pci_drv.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_plane.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_regs.h > [...] > >> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.c b/drivers/gpu/drm/lsdc/lsdc_i2c.c >> new file mode 100644 >> index 000000000000..55beed9266fa >> --- /dev/null >> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.c >> @@ -0,0 +1,235 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * KMS driver for Loongson display controller > Not really a useful comment since every file has the same one. > >> + * Copyright (C) 2022 Loongson Corporation >> + */ >> + >> +/* >> + * Authors: >> + * Sui Jingfeng <suijingfeng@loongson.cn> >> + */ >> + >> +#include <linux/i2c.h> >> +#include <linux/pci.h> >> + >> +#include "lsdc_drv.h" >> +#include "lsdc_regs.h" >> +#include "lsdc_i2c.h" >> + >> +/* >> + * ls7a_gpio_i2c_set - set the state of a gpio pin indicated by mask >> + * @mask: gpio pin mask >> + */ >> +static void ls7a_gpio_i2c_set(struct lsdc_i2c * const li2c, int mask, int state) >> +{ >> + unsigned long flags; >> + u8 val; >> + >> + spin_lock_irqsave(&li2c->reglock, flags); > What are you protecting? Doesn't the caller serialize calls to these > functions? > >> + >> + if (state) { >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + } else { >> + val = readb(li2c->dir_reg); >> + val &= ~mask; >> + writeb(val, li2c->dir_reg); >> + >> + val = readb(li2c->dat_reg); >> + if (state) > This condition is never true. We're in the 'else' because !state. > >> + val |= mask; >> + else >> + val &= ~mask; >> + writeb(val, li2c->dat_reg); > Shouldn't you set the data register low first and then change the > direction? Otherwise, you may be driving high for a moment. However, if > high is always done by setting the direction as input, why write the > data register each time? I'm assuming whatever is written to the dat_reg > is maintained regardless of pin state. > When the pin is input, i am not sure value written to it will be preserved. I'm worry about it get flushed by the external input value. Because the output data register is same with the input data register( offset is 0x1650). The hardware designer do not provided a separation. >> + } >> + >> + spin_unlock_irqrestore(&li2c->reglock, flags); >> +} >> + >> +/* >> + * ls7a_gpio_i2c_get - read value back from gpio pin >> + * @mask: gpio pin mask >> + */ >> +static int ls7a_gpio_i2c_get(struct lsdc_i2c * const li2c, int mask) >> +{ >> + unsigned long flags; >> + u8 val; >> + >> + spin_lock_irqsave(&li2c->reglock, flags); >> + >> + /* first set this pin as input */ >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + >> + /* then get level state from this pin */ >> + val = readb(li2c->dat_reg); >> + >> + spin_unlock_irqrestore(&li2c->reglock, flags); >> + >> + return (val & mask) ? 1 : 0; >> +} >> + >> +/* set the state on the i2c->sda pin */ >> +static void ls7a_i2c_set_sda(void *i2c, int state) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_set(li2c, li2c->sda, state); >> +} >> + >> +/* set the state on the i2c->scl pin */ >> +static void ls7a_i2c_set_scl(void *i2c, int state) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_set(li2c, li2c->scl, state); >> +} >> + >> +/* read the value from the i2c->sda pin */ >> +static int ls7a_i2c_get_sda(void *i2c) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_get(li2c, li2c->sda); >> +} >> + >> +/* read the value from the i2c->scl pin */ >> +static int ls7a_i2c_get_scl(void *i2c) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_get(li2c, li2c->scl); >> +} >> + >> +/* >> + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c >> + * >> + * @index : output channel index, 0 for DVO0, 1 for DVO1 >> + */ >> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) >> +{ >> + char compat[32] = {0}; >> + unsigned int udelay = 5; >> + unsigned int timeout = 2200; >> + int nr = -1; >> + struct i2c_adapter *adapter; >> + struct lsdc_i2c *li2c; >> + struct device_node *i2c_np; >> + int ret; >> + >> + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); >> + if (!li2c) >> + return ERR_PTR(-ENOMEM); >> + >> + li2c->index = index; >> + li2c->dev = dev; >> + >> + if (index == 0) { >> + li2c->sda = 0x01; >> + li2c->scl = 0x02; >> + } else if (index == 1) { >> + li2c->sda = 0x04; >> + li2c->scl = 0x08; > Just require this to be in DT rather than having some default. > >> + } >> + >> + spin_lock_init(&li2c->reglock); >> + >> + snprintf(compat, sizeof(compat), "lsdc,i2c-gpio-%d", index); > compatible values shouldn't have an index and you shouldn't need a > index in DT. You need to iterate over child nodes with matching > compatible. > >> + i2c_np = of_find_compatible_node(dev->of_node, NULL, compat); >> + if (i2c_np) { >> + u32 sda, scl; >> + >> + dev_dbg(dev, "Has %s property in the DT", compat); >> + >> + /* */ >> + ret = of_property_read_u32(i2c_np, "sda", &sda); > Custom properties need a vendor prefix. > >> + if (ret == 0) >> + li2c->sda = 1 << sda; >> + >> + ret = of_property_read_u32(i2c_np, "scl", &scl); >> + if (ret == 0) >> + li2c->scl = 1 << scl; >> + >> + /* Optional properties which made the driver more flexible */ >> + of_property_read_u32(i2c_np, "udelay", &udelay); >> + of_property_read_u32(i2c_np, "timeout", &timeout); > These aren't documented. Do you really need them in DT? Yes, in very rare case: When debugging, sometimes one way I2C works, another way I2C not on specific board. and you want to see what will happen if you change it from 5 to 2. modify device tree is enough, have to recompile the kernel and driver modules every time. It is optional through. Please do not ask me to document such a easy thing, DT itself is a documention, human readable, it already speak for itself. >> + of_property_read_u32(i2c_np, "reg", &nr); >> + } >> + >> + dev_dbg(dev, "%s: sda=%u, scl=%u, nr=%d, udelay=%u, timeout=%u\n", >> + compat, li2c->sda, li2c->scl, nr, udelay, timeout); >> + >> + li2c->reg_base = base; >> + >> + li2c->dir_reg = li2c->reg_base + LS7A_DC_GPIO_DIR_REG; >> + li2c->dat_reg = li2c->reg_base + LS7A_DC_GPIO_DAT_REG; >> + >> + li2c->bit.setsda = ls7a_i2c_set_sda; >> + li2c->bit.setscl = ls7a_i2c_set_scl; >> + li2c->bit.getsda = ls7a_i2c_get_sda; >> + li2c->bit.getscl = ls7a_i2c_get_scl; >> + li2c->bit.udelay = udelay; >> + li2c->bit.timeout = usecs_to_jiffies(timeout); >> + li2c->bit.data = li2c; >> + >> + adapter = &li2c->adapter; >> + adapter->algo_data = &li2c->bit; >> + adapter->owner = THIS_MODULE; >> + adapter->class = I2C_CLASS_DDC; >> + adapter->dev.parent = dev; >> + adapter->nr = nr; >> + if (i2c_np) { >> + adapter->dev.of_node = i2c_np; >> + of_node_put(i2c_np); >> + } >> + >> + strscpy(adapter->name, &compat[5], sizeof(adapter->name)); >> + >> + i2c_set_adapdata(adapter, li2c); >> + >> + ret = i2c_bit_add_numbered_bus(adapter); > Why do you care what the bus number is? You shouldn't need to. > >> + if (ret) { >> + if (i2c_np) >> + of_node_put(i2c_np); >> + >> + devm_kfree(dev, li2c); >> + return ERR_PTR(ret); >> + } >> + >> + return li2c; >> +} >> + >> +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c) >> +{ >> + struct i2c_adapter *adapter; >> + >> + if (li2c) { >> + adapter = &li2c->adapter; >> + >> + if (adapter && adapter->dev.of_node) >> + of_node_put(adapter->dev.of_node); >> + >> + devm_kfree(ddev->dev, li2c); >> + } >> +} >> + >> +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, >> + unsigned int index) >> +{ >> + const struct lsdc_chip_desc * const descp = ldev->desc; >> + struct lsdc_i2c *li2c; >> + >> + if (index >= descp->num_of_crtc) { >> + drm_err(ldev->ddev, "I2c adapter is no more than %u, %u\n", >> + descp->num_of_crtc, index); >> + return NULL; >> + } >> + >> + li2c = ldev->li2c[index]; >> + if (li2c) >> + return &li2c->adapter; >> + >> + return NULL; >> +} >> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.h b/drivers/gpu/drm/lsdc/lsdc_i2c.h >> new file mode 100644 >> index 000000000000..4ab825143eb4 >> --- /dev/null >> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.h >> @@ -0,0 +1,42 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * KMS driver for Loongson display controller >> + * Copyright (C) 2022 Loongson Corporation >> + */ >> + >> +/* >> + * Authors: >> + * Sui Jingfeng <suijingfeng@loongson.cn> >> + */ >> + >> +#ifndef __LSDC_I2C__ >> +#define __LSDC_I2C__ >> + >> +#include <linux/i2c.h> >> +#include <linux/i2c-algo-bit.h> >> +#include <linux/pci.h> >> + >> +struct lsdc_i2c { >> + struct device *dev; >> + struct i2c_adapter adapter; >> + struct i2c_algo_bit_data bit; >> + /* @reglock: protects concurrent register access */ >> + spinlock_t reglock; >> + void __iomem *reg_base; >> + void __iomem *dir_reg; >> + void __iomem *dat_reg; >> + int index; >> + /* pin bit mask */ >> + u8 sda; >> + u8 scl; >> +}; >> + >> +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c); >> + >> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, >> + void *base, >> + unsigned int index); >> + >> +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, >> + unsigned int index); >> +#endif
On 2022/3/23 21:11, Rob Herring wrote: > On Wed, Mar 23, 2022 at 12:12:43PM +0800, Sui Jingfeng wrote: >> On 2022/3/23 04:49, Rob Herring wrote: >>>> +/* >>>> + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c >>>> + * >>>> + * @index : output channel index, 0 for DVO0, 1 for DVO1 >>>> + */ >>>> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) >>>> +{ >>>> + char compat[32] = {0}; >>>> + unsigned int udelay = 5; >>>> + unsigned int timeout = 2200; >>>> + int nr = -1; >>>> + struct i2c_adapter *adapter; >>>> + struct lsdc_i2c *li2c; >>>> + struct device_node *i2c_np; >>>> + int ret; >>>> + >>>> + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); >>>> + if (!li2c) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + li2c->index = index; >>>> + li2c->dev = dev; >>>> + >>>> + if (index == 0) { >>>> + li2c->sda = 0x01; >>>> + li2c->scl = 0x02; >>>> + } else if (index == 1) { >>>> + li2c->sda = 0x04; >>>> + li2c->scl = 0x08; >>> Just require this to be in DT rather than having some default. >>> >> By design, I am try very hard to let the code NOT fully DT dependent. DT is nice , easy to learn and use. >> But kernel side developer plan to follow UEFI + ACPI Specification on LS3A5000 + LS7A1000 platform. See [1] >> There will no DT support then, provide a convention support make the driver more flexible. I want the >> driver works with minimal requirement. The driver just works on simple boards by put the following dc device >> node in arch/mips/dts/loongson/loongson64g_4core_ls7a.dts, > Pick DT or ACPI for the platform, not both. We don't need to have both > in the kernel to support. > > Rob Hi Rob, We can only choose DT currently, we love DT, but it is kernel side developer's choice. We just avoid deep coupling which tend to lost flexibility. All I can and should do is make the drivers works, writing code beautiful does not means it can works like a charm. From what i am understanding, DT is not a strict specification, but in return flexible. Force every driver comply with what already have is tend to prohibit innovation. It just too late to do so.
On 2022/3/23 04:49, Rob Herring wrote: >> + } >> + >> + spin_lock_init(&li2c->reglock); >> + >> + snprintf(compat, sizeof(compat), "lsdc,i2c-gpio-%d", index); > compatible values shouldn't have an index and you shouldn't need a > index in DT. You need to iterate over child nodes with matching > compatible. Why compatible values shouldn't have an index, does devicetree specification prohibit this? [1] The recommended format is "manufacturer,model", where manufacturer is a string describing the name of the manufacturer (such as a stock ticker symbol), and model specifies the model number. [1] [1] https://www.devicetree.org/specifications/
On Thu, Mar 24, 2022 at 09:39:49AM +0800, Sui Jingfeng wrote: > > On 2022/3/23 04:49, Rob Herring wrote: > > On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote: > > > From: suijingfeng <suijingfeng@loongson.cn> > > > > > > There is a display controller in loongson's LS2K1000 SoC and LS7A1000 > > > bridge chip, the display controller is a PCI device in those chips. It > > > has two display pipes but with only one hardware cursor. Each way has > > > a DVO interface which provide RGB888 signals, vertical & horizontal > > > synchronisations, data enable and the pixel clock. Each CRTC is able to > > > scanout from 1920x1080 resolution at 60Hz, the maxmium resolution is > > > 2048x2048 according to the hardware spec. Loongson display controllers > > > are simple which require scanout buffers to be physically contiguous. [...] > > > + val |= mask; > > > + else > > > + val &= ~mask; > > > + writeb(val, li2c->dat_reg); > > Shouldn't you set the data register low first and then change the > > direction? Otherwise, you may be driving high for a moment. However, if > > high is always done by setting the direction as input, why write the > > data register each time? I'm assuming whatever is written to the dat_reg > > is maintained regardless of pin state. > > > When the pin is input, i am not sure value written to it will be preserved. > > I'm worry about it get flushed by the external input value. > > Because the output data register is same with the input data register( > offset is 0x1650). > > The hardware designer do not provided a separation. Usually for GPIO data registers the read value is current pin state regardless of direction and the written value is what to drive as an output. But your h/w could be different. > > > + > > > + /* Optional properties which made the driver more flexible */ > > > + of_property_read_u32(i2c_np, "udelay", &udelay); > > > + of_property_read_u32(i2c_np, "timeout", &timeout); > > These aren't documented. Do you really need them in DT? > > Yes, in very rare case: > > When debugging, sometimes one way I2C works, another way I2C not on specific > board. This is not specific to you, so why do you solve it in a way that only works for you? If you want to add tuning parameters to the i2c bit algorithm, why don't you do so in a way that works for all users? I'm sure the I2C maintainer and others have some opinion on this, but they'll never see it hidden away in some display driver. > and you want to see what will happen if you change it from 5 to 2. > > modify device tree is enough, have to recompile the kernel and driver > modules every time. Modifying the DT is not the easiest way to debug either. > It is optional through. Lots of properties are optional, what's your point? > Please do not ask me to document such a easy thing, Everything must be documented. There's nothing more to discuss. > DT itself is a documention, human readable, it already speak for itself. It is machine readable too. Undocumented properties generate warnings now. Rob
On Thu, Mar 24, 2022 at 03:32:01PM +0800, Sui Jingfeng wrote: > > On 2022/3/23 04:49, Rob Herring wrote: > > > + } > > > + > > > + spin_lock_init(&li2c->reglock); > > > + > > > + snprintf(compat, sizeof(compat), "lsdc,i2c-gpio-%d", index); > > compatible values shouldn't have an index and you shouldn't need a > > index in DT. You need to iterate over child nodes with matching > > compatible. > > Why compatible values shouldn't have an index, does devicetree > specification prohibit this? [1] Probably not explicitly, but that's fundamentally not how compatible works. 'compatible' defines WHAT the device is, not WHICH device and that is used for matching devices to drivers. Drivers work on multiple instances. > The recommended format is "manufacturer,model", where manufacturer is a string describing the name > of the manufacturer (such as a stock ticker symbol), and model specifies the model number. [1] I don't see anything saying to put the instance in there, do you? > > [1] https://www.devicetree.org/specifications/ >
On 2022/3/23 21:11, Rob Herring wrote: > On Wed, Mar 23, 2022 at 12:12:43PM +0800, Sui Jingfeng wrote: >> On 2022/3/23 04:49, Rob Herring wrote: >>>> +/* >>>> + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c >>>> + * >>>> + * @index : output channel index, 0 for DVO0, 1 for DVO1 >>>> + */ >>>> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) >>>> +{ >>>> + char compat[32] = {0}; >>>> + unsigned int udelay = 5; >>>> + unsigned int timeout = 2200; >>>> + int nr = -1; >>>> + struct i2c_adapter *adapter; >>>> + struct lsdc_i2c *li2c; >>>> + struct device_node *i2c_np; >>>> + int ret; >>>> + >>>> + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); >>>> + if (!li2c) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + li2c->index = index; >>>> + li2c->dev = dev; >>>> + >>>> + if (index == 0) { >>>> + li2c->sda = 0x01; >>>> + li2c->scl = 0x02; >>>> + } else if (index == 1) { >>>> + li2c->sda = 0x04; >>>> + li2c->scl = 0x08; >>> Just require this to be in DT rather than having some default. >>> >> By design, I am try very hard to let the code NOT fully DT dependent. DT is nice , easy to learn and use. >> But kernel side developer plan to follow UEFI + ACPI Specification on LS3A5000 + LS7A1000 platform. See [1] >> There will no DT support then, provide a convention support make the driver more flexible. I want the >> driver works with minimal requirement. The driver just works on simple boards by put the following dc device >> node in arch/mips/dts/loongson/loongson64g_4core_ls7a.dts, > Pick DT or ACPI for the platform, not both. We don't need to have both > in the kernel to support. > > Rob Hi, everybody I have send new version of my patch, there may still have flaws though. Would you like to help to review it again? https://patchwork.freedesktop.org/series/102104/ @Rob @Maxime @Krzysztof I have correct many issues as you guys mentioned before, if something get ignored and I may miss the point, would like to mention it again on my new patches? because mails received previously got lost(flushed by new mails). I can only reply to new reviews. Thanks for your time.
On 2022/3/23 04:49, Rob Herring wrote: > On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote: >> From: suijingfeng <suijingfeng@loongson.cn> >> >> There is a display controller in loongson's LS2K1000 SoC and LS7A1000 >> bridge chip, the display controller is a PCI device in those chips. It >> has two display pipes but with only one hardware cursor. Each way has >> a DVO interface which provide RGB888 signals, vertical & horizontal >> synchronisations, data enable and the pixel clock. Each CRTC is able to >> scanout from 1920x1080 resolution at 60Hz, the maxmium resolution is >> 2048x2048 according to the hardware spec. Loongson display controllers >> are simple which require scanout buffers to be physically contiguous. >> >> For LS7A1000 bridge chip, the DC is equipped with a dedicated video RAM >> which is typically 64MB or more. In this case, VRAM helper based driver >> is intend to be used. While LS2K1000 is a SoC, only system memory is >> available. Therefore CMA helper based driver is intend to be used. It is >> possible to use VRAM helper based solution by carving out part of system >> memory as VRAM though. >> >> For LS7A1000, there are 4 dedicated GPIOs whose control register is >> located at the DC register space, They are used to emulate two way i2c. >> One for DVO0, another for DVO1. LS2K1000 and LS2K0500 SoC don't have such >> GPIO hardwared, they grab i2c adapter from other module, either general >> purpose GPIO emulated i2c or hardware i2c adapter. >> >> +------+ +-----------------------------------+ >> | DDR4 | | +-------------------+ | >> +------+ | | PCIe Root complex | LS7A1000 | >> || MC0 | +--++---------++----+ | >> +----------+ HT 3.0 | || || | >> | LS3A4000 |<-------->| +---++---+ +--++--+ +---------+ +------+ >> | CPU |<-------->| | GC1000 | | LSDC |<-->| DDR3 MC |<->| VRAM | >> +----------+ | +--------+ +-+--+-+ +---------+ +------+ >> || MC1 +---------------|--|----------------+ >> +------+ | | >> | DDR4 | +-------+ DVO0 | | DVO1 +------+ >> +------+ VGA <--|ADV7125|<--------+ +-------->|TFP410|--> DVI/HDMI >> +-------+ +------+ >> >> The above picture give a simple usage of LS7A1000, note that the encoder >> is not necessary adv7125 or tfp410, other candicates can be ch7034b, >> sil9022, ite66121 and lt8618 etc. >> >> v2: Fixup warnings reported by kernel test robot >> >> v3: Fix more grammar mistakes in Kconfig reported by Randy Dunlap and give >> more details about lsdc. >> >> v4: >> 1) Add dts required and explain why device tree is required. >> 2) Give more description about lsdc and VRAM helper based driver. >> 3) Fix warnings reported by kernel test robot. >> 4) Introduce stride_alignment member into struct lsdc_chip_desc, the >> stride alignment is 256 bytes for ls7a1000, ls2k1000 and ls2k0500. >> >> v5: >> 1) Using writel and readl replace writeq and readq, to fix kernel test >> robot report build error on other archtecture. >> 2) Set default fb format to XRGB8888 at crtc reset time. >> >> v6: >> 1) Explain why we are not switch to drm dridge subsystem on ls2k1000. >> 2) Explain why tiny drm driver is not suitable for us. >> 3) Give a short description of the trival dirty uppdate implement based >> on CMA helper. >> >> v7: >> 1) Remove select I2C_GPIO and I2C_LS2X in Kconfig, it is not ready now >> 2) Licensing issues are fixed suggested by Krzysztof Kozlowski. >> 3) Remove lsdc_pixpll_print(), part of it move to debugfs. >> 4) Set prefer_shadow to true if vram based driver is in using. >> 5) Replace double blank lines with single line in all files. >> 6) Verbose cmd line parameter is replaced with drm_dbg() >> 7) All warnnings reported by ./scripts/checkpatch.pl --strict are fixed >> 8) Get edid from dtb support is removed as suggested by Maxime Ripard >> 9) Fix typos and various improvement >> >> v8: >> 1) Drop damage update implement and its command line. >> 2) Drop DRM_LSDC_VRAM_DRIVER config option as suggested by Maxime. >> 3) Deduce DC's identification from its compatible property. >> 4) Drop the board specific dts patch. >> 5) Add documention about the display controller device node. >> >> v9: >> 1) Fix the warnings reported by checkpatch script and fix typos >> >> v10: >> 1) Pass `make dt_binding_check` validation >> 2) Fix warnings reported by kernel test robot >> >> v11: >> 1) Convert the driver to use drm bridge and of graph framework. >> 2) Dump register value support through debugfs. >> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: suijingfeng <suijingfeng@loongson.cn> >> Signed-off-by: Sui Jingfeng <15330273260@189.cn> >> Signed-off-by: suijingfeng <suijingfeng@loongson.cn> >> --- >> drivers/gpu/drm/Kconfig | 2 + >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/lsdc/Kconfig | 23 ++ >> drivers/gpu/drm/lsdc/Makefile | 13 + >> drivers/gpu/drm/lsdc/lsdc_crtc.c | 396 +++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_drv.c | 547 ++++++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_drv.h | 197 ++++++++++ >> drivers/gpu/drm/lsdc/lsdc_i2c.c | 235 ++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_i2c.h | 42 ++ >> drivers/gpu/drm/lsdc/lsdc_irq.c | 58 +++ >> drivers/gpu/drm/lsdc/lsdc_irq.h | 18 + >> drivers/gpu/drm/lsdc/lsdc_output.c | 262 +++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_output.h | 24 ++ >> drivers/gpu/drm/lsdc/lsdc_pci_drv.c | 328 ++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_plane.c | 470 +++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_pll.c | 574 ++++++++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_pll.h | 88 +++++ >> drivers/gpu/drm/lsdc/lsdc_regs.h | 220 +++++++++++ >> 18 files changed, 3498 insertions(+) >> create mode 100644 drivers/gpu/drm/lsdc/Kconfig >> create mode 100644 drivers/gpu/drm/lsdc/Makefile >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_crtc.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pci_drv.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_plane.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_regs.h > [...] > >> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.c b/drivers/gpu/drm/lsdc/lsdc_i2c.c >> new file mode 100644 >> index 000000000000..55beed9266fa >> --- /dev/null >> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.c >> @@ -0,0 +1,235 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * KMS driver for Loongson display controller > Not really a useful comment since every file has the same one. > >> + * Copyright (C) 2022 Loongson Corporation >> + */ >> + >> +/* >> + * Authors: >> + * Sui Jingfeng <suijingfeng@loongson.cn> >> + */ >> + >> +#include <linux/i2c.h> >> +#include <linux/pci.h> >> + >> +#include "lsdc_drv.h" >> +#include "lsdc_regs.h" >> +#include "lsdc_i2c.h" >> + >> +/* >> + * ls7a_gpio_i2c_set - set the state of a gpio pin indicated by mask >> + * @mask: gpio pin mask >> + */ >> +static void ls7a_gpio_i2c_set(struct lsdc_i2c * const li2c, int mask, int state) >> +{ >> + unsigned long flags; >> + u8 val; >> + >> + spin_lock_irqsave(&li2c->reglock, flags); > What are you protecting? Doesn't the caller serialize calls to these > functions? Hi, there are some old ls7a1000 bridge chip product still in use at china market, The display controller in old ls7a1000 bridge chip does not support concurrent register access properly, when two or more threads writing the dc registers at the same time, the ls7a1000 bridge chip will hung. But the hung only happen at low occurrence, wrap register access with spin lock is more stable in practice for old ls7a1000 bridge chip. >> + >> + if (state) { >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + } else { >> + val = readb(li2c->dir_reg); >> + val &= ~mask; >> + writeb(val, li2c->dir_reg); >> + >> + val = readb(li2c->dat_reg); >> + if (state) > This condition is never true. We're in the 'else' because !state. > >> + val |= mask; >> + else >> + val &= ~mask; >> + writeb(val, li2c->dat_reg); > Shouldn't you set the data register low first and then change the > direction? Otherwise, you may be driving high for a moment. However, if > high is always done by setting the direction as input, why write the > data register each time? I'm assuming whatever is written to the dat_reg > is maintained regardless of pin state. > >> + } >> + >> + spin_unlock_irqrestore(&li2c->reglock, flags); >> +} >> + >> +/* >> + * ls7a_gpio_i2c_get - read value back from gpio pin >> + * @mask: gpio pin mask >> + */ >> +static int ls7a_gpio_i2c_get(struct lsdc_i2c * const li2c, int mask) >> +{ >> + unsigned long flags; >> + u8 val; >> + >> + spin_lock_irqsave(&li2c->reglock, flags); >> + >> + /* first set this pin as input */ >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + >> + /* then get level state from this pin */ >> + val = readb(li2c->dat_reg); >> + >> + spin_unlock_irqrestore(&li2c->reglock, flags); >> + >> + return (val & mask) ? 1 : 0; >> +} >> + >> +/* set the state on the i2c->sda pin */ >> +static void ls7a_i2c_set_sda(void *i2c, int state) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_set(li2c, li2c->sda, state); >> +} >> + >> +/* set the state on the i2c->scl pin */ >> +static void ls7a_i2c_set_scl(void *i2c, int state) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_set(li2c, li2c->scl, state); >> +} >> + >> +/* read the value from the i2c->sda pin */ >> +static int ls7a_i2c_get_sda(void *i2c) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_get(li2c, li2c->sda); >> +} >> + >> +/* read the value from the i2c->scl pin */ >> +static int ls7a_i2c_get_scl(void *i2c) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_get(li2c, li2c->scl); >> +} >> + >> +/* >> + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c >> + * >> + * @index : output channel index, 0 for DVO0, 1 for DVO1 >> + */ >> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) >> +{ >> + char compat[32] = {0}; >> + unsigned int udelay = 5; >> + unsigned int timeout = 2200; >> + int nr = -1; >> + struct i2c_adapter *adapter; >> + struct lsdc_i2c *li2c; >> + struct device_node *i2c_np; >> + int ret; >> + >> + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); >> + if (!li2c) >> + return ERR_PTR(-ENOMEM); >> + >> + li2c->index = index; >> + li2c->dev = dev; >> + >> + if (index == 0) { >> + li2c->sda = 0x01; >> + li2c->scl = 0x02; >> + } else if (index == 1) { >> + li2c->sda = 0x04; >> + li2c->scl = 0x08; > Just require this to be in DT rather than having some default. > >> + } >> + >> + spin_lock_init(&li2c->reglock); >> + >> + snprintf(compat, sizeof(compat), "lsdc,i2c-gpio-%d", index); > compatible values shouldn't have an index and you shouldn't need a > index in DT. You need to iterate over child nodes with matching > compatible. > >> + i2c_np = of_find_compatible_node(dev->of_node, NULL, compat); >> + if (i2c_np) { >> + u32 sda, scl; >> + >> + dev_dbg(dev, "Has %s property in the DT", compat); >> + >> + /* */ >> + ret = of_property_read_u32(i2c_np, "sda", &sda); > Custom properties need a vendor prefix. > >> + if (ret == 0) >> + li2c->sda = 1 << sda; >> + >> + ret = of_property_read_u32(i2c_np, "scl", &scl); >> + if (ret == 0) >> + li2c->scl = 1 << scl; >> + >> + /* Optional properties which made the driver more flexible */ >> + of_property_read_u32(i2c_np, "udelay", &udelay); >> + of_property_read_u32(i2c_np, "timeout", &timeout); > These aren't documented. Do you really need them in DT? > >> + of_property_read_u32(i2c_np, "reg", &nr); >> + } >> + >> + dev_dbg(dev, "%s: sda=%u, scl=%u, nr=%d, udelay=%u, timeout=%u\n", >> + compat, li2c->sda, li2c->scl, nr, udelay, timeout); >> + >> + li2c->reg_base = base; >> + >> + li2c->dir_reg = li2c->reg_base + LS7A_DC_GPIO_DIR_REG; >> + li2c->dat_reg = li2c->reg_base + LS7A_DC_GPIO_DAT_REG; >> + >> + li2c->bit.setsda = ls7a_i2c_set_sda; >> + li2c->bit.setscl = ls7a_i2c_set_scl; >> + li2c->bit.getsda = ls7a_i2c_get_sda; >> + li2c->bit.getscl = ls7a_i2c_get_scl; >> + li2c->bit.udelay = udelay; >> + li2c->bit.timeout = usecs_to_jiffies(timeout); >> + li2c->bit.data = li2c; >> + >> + adapter = &li2c->adapter; >> + adapter->algo_data = &li2c->bit; >> + adapter->owner = THIS_MODULE; >> + adapter->class = I2C_CLASS_DDC; >> + adapter->dev.parent = dev; >> + adapter->nr = nr; >> + if (i2c_np) { >> + adapter->dev.of_node = i2c_np; >> + of_node_put(i2c_np); >> + } >> + >> + strscpy(adapter->name, &compat[5], sizeof(adapter->name)); >> + >> + i2c_set_adapdata(adapter, li2c); >> + >> + ret = i2c_bit_add_numbered_bus(adapter); > Why do you care what the bus number is? You shouldn't need to. > >> + if (ret) { >> + if (i2c_np) >> + of_node_put(i2c_np); >> + >> + devm_kfree(dev, li2c); >> + return ERR_PTR(ret); >> + } >> + >> + return li2c; >> +} >> + >> +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c) >> +{ >> + struct i2c_adapter *adapter; >> + >> + if (li2c) { >> + adapter = &li2c->adapter; >> + >> + if (adapter && adapter->dev.of_node) >> + of_node_put(adapter->dev.of_node); >> + >> + devm_kfree(ddev->dev, li2c); >> + } >> +} >> + >> +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, >> + unsigned int index) >> +{ >> + const struct lsdc_chip_desc * const descp = ldev->desc; >> + struct lsdc_i2c *li2c; >> + >> + if (index >= descp->num_of_crtc) { >> + drm_err(ldev->ddev, "I2c adapter is no more than %u, %u\n", >> + descp->num_of_crtc, index); >> + return NULL; >> + } >> + >> + li2c = ldev->li2c[index]; >> + if (li2c) >> + return &li2c->adapter; >> + >> + return NULL; >> +} >> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.h b/drivers/gpu/drm/lsdc/lsdc_i2c.h >> new file mode 100644 >> index 000000000000..4ab825143eb4 >> --- /dev/null >> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.h >> @@ -0,0 +1,42 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * KMS driver for Loongson display controller >> + * Copyright (C) 2022 Loongson Corporation >> + */ >> + >> +/* >> + * Authors: >> + * Sui Jingfeng <suijingfeng@loongson.cn> >> + */ >> + >> +#ifndef __LSDC_I2C__ >> +#define __LSDC_I2C__ >> + >> +#include <linux/i2c.h> >> +#include <linux/i2c-algo-bit.h> >> +#include <linux/pci.h> >> + >> +struct lsdc_i2c { >> + struct device *dev; >> + struct i2c_adapter adapter; >> + struct i2c_algo_bit_data bit; >> + /* @reglock: protects concurrent register access */ >> + spinlock_t reglock; >> + void __iomem *reg_base; >> + void __iomem *dir_reg; >> + void __iomem *dat_reg; >> + int index; >> + /* pin bit mask */ >> + u8 sda; >> + u8 scl; >> +}; >> + >> +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c); >> + >> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, >> + void *base, >> + unsigned int index); >> + >> +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, >> + unsigned int index); >> +#endif
On 2022/3/23 04:49, Rob Herring wrote: > On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote: >> From: suijingfeng <suijingfeng@loongson.cn> >> >> There is a display controller in loongson's LS2K1000 SoC and LS7A1000 >> bridge chip, the display controller is a PCI device in those chips. It >> has two display pipes but with only one hardware cursor. Each way has >> a DVO interface which provide RGB888 signals, vertical & horizontal >> synchronisations, data enable and the pixel clock. Each CRTC is able to >> scanout from 1920x1080 resolution at 60Hz, the maxmium resolution is >> 2048x2048 according to the hardware spec. Loongson display controllers >> are simple which require scanout buffers to be physically contiguous. >> >> For LS7A1000 bridge chip, the DC is equipped with a dedicated video RAM >> which is typically 64MB or more. In this case, VRAM helper based driver >> is intend to be used. While LS2K1000 is a SoC, only system memory is >> available. Therefore CMA helper based driver is intend to be used. It is >> possible to use VRAM helper based solution by carving out part of system >> memory as VRAM though. >> >> For LS7A1000, there are 4 dedicated GPIOs whose control register is >> located at the DC register space, They are used to emulate two way i2c. >> One for DVO0, another for DVO1. LS2K1000 and LS2K0500 SoC don't have such >> GPIO hardwared, they grab i2c adapter from other module, either general >> purpose GPIO emulated i2c or hardware i2c adapter. >> >> +------+ +-----------------------------------+ >> | DDR4 | | +-------------------+ | >> +------+ | | PCIe Root complex | LS7A1000 | >> || MC0 | +--++---------++----+ | >> +----------+ HT 3.0 | || || | >> | LS3A4000 |<-------->| +---++---+ +--++--+ +---------+ +------+ >> | CPU |<-------->| | GC1000 | | LSDC |<-->| DDR3 MC |<->| VRAM | >> +----------+ | +--------+ +-+--+-+ +---------+ +------+ >> || MC1 +---------------|--|----------------+ >> +------+ | | >> | DDR4 | +-------+ DVO0 | | DVO1 +------+ >> +------+ VGA <--|ADV7125|<--------+ +-------->|TFP410|--> DVI/HDMI >> +-------+ +------+ >> >> The above picture give a simple usage of LS7A1000, note that the encoder >> is not necessary adv7125 or tfp410, other candicates can be ch7034b, >> sil9022, ite66121 and lt8618 etc. >> >> v2: Fixup warnings reported by kernel test robot >> >> v3: Fix more grammar mistakes in Kconfig reported by Randy Dunlap and give >> more details about lsdc. >> >> v4: >> 1) Add dts required and explain why device tree is required. >> 2) Give more description about lsdc and VRAM helper based driver. >> 3) Fix warnings reported by kernel test robot. >> 4) Introduce stride_alignment member into struct lsdc_chip_desc, the >> stride alignment is 256 bytes for ls7a1000, ls2k1000 and ls2k0500. >> >> v5: >> 1) Using writel and readl replace writeq and readq, to fix kernel test >> robot report build error on other archtecture. >> 2) Set default fb format to XRGB8888 at crtc reset time. >> >> v6: >> 1) Explain why we are not switch to drm dridge subsystem on ls2k1000. >> 2) Explain why tiny drm driver is not suitable for us. >> 3) Give a short description of the trival dirty uppdate implement based >> on CMA helper. >> >> v7: >> 1) Remove select I2C_GPIO and I2C_LS2X in Kconfig, it is not ready now >> 2) Licensing issues are fixed suggested by Krzysztof Kozlowski. >> 3) Remove lsdc_pixpll_print(), part of it move to debugfs. >> 4) Set prefer_shadow to true if vram based driver is in using. >> 5) Replace double blank lines with single line in all files. >> 6) Verbose cmd line parameter is replaced with drm_dbg() >> 7) All warnnings reported by ./scripts/checkpatch.pl --strict are fixed >> 8) Get edid from dtb support is removed as suggested by Maxime Ripard >> 9) Fix typos and various improvement >> >> v8: >> 1) Drop damage update implement and its command line. >> 2) Drop DRM_LSDC_VRAM_DRIVER config option as suggested by Maxime. >> 3) Deduce DC's identification from its compatible property. >> 4) Drop the board specific dts patch. >> 5) Add documention about the display controller device node. >> >> v9: >> 1) Fix the warnings reported by checkpatch script and fix typos >> >> v10: >> 1) Pass `make dt_binding_check` validation >> 2) Fix warnings reported by kernel test robot >> >> v11: >> 1) Convert the driver to use drm bridge and of graph framework. >> 2) Dump register value support through debugfs. >> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: suijingfeng <suijingfeng@loongson.cn> >> Signed-off-by: Sui Jingfeng <15330273260@189.cn> >> Signed-off-by: suijingfeng <suijingfeng@loongson.cn> >> --- >> drivers/gpu/drm/Kconfig | 2 + >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/lsdc/Kconfig | 23 ++ >> drivers/gpu/drm/lsdc/Makefile | 13 + >> drivers/gpu/drm/lsdc/lsdc_crtc.c | 396 +++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_drv.c | 547 ++++++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_drv.h | 197 ++++++++++ >> drivers/gpu/drm/lsdc/lsdc_i2c.c | 235 ++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_i2c.h | 42 ++ >> drivers/gpu/drm/lsdc/lsdc_irq.c | 58 +++ >> drivers/gpu/drm/lsdc/lsdc_irq.h | 18 + >> drivers/gpu/drm/lsdc/lsdc_output.c | 262 +++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_output.h | 24 ++ >> drivers/gpu/drm/lsdc/lsdc_pci_drv.c | 328 ++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_plane.c | 470 +++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_pll.c | 574 ++++++++++++++++++++++++++++ >> drivers/gpu/drm/lsdc/lsdc_pll.h | 88 +++++ >> drivers/gpu/drm/lsdc/lsdc_regs.h | 220 +++++++++++ >> 18 files changed, 3498 insertions(+) >> create mode 100644 drivers/gpu/drm/lsdc/Kconfig >> create mode 100644 drivers/gpu/drm/lsdc/Makefile >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_crtc.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pci_drv.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_plane.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.c >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.h >> create mode 100644 drivers/gpu/drm/lsdc/lsdc_regs.h > [...] > >> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.c b/drivers/gpu/drm/lsdc/lsdc_i2c.c >> new file mode 100644 >> index 000000000000..55beed9266fa >> --- /dev/null >> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.c >> @@ -0,0 +1,235 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * KMS driver for Loongson display controller > Not really a useful comment since every file has the same one. > >> + * Copyright (C) 2022 Loongson Corporation >> + */ >> + >> +/* >> + * Authors: >> + * Sui Jingfeng <suijingfeng@loongson.cn> >> + */ >> + >> +#include <linux/i2c.h> >> +#include <linux/pci.h> >> + >> +#include "lsdc_drv.h" >> +#include "lsdc_regs.h" >> +#include "lsdc_i2c.h" >> + >> +/* >> + * ls7a_gpio_i2c_set - set the state of a gpio pin indicated by mask >> + * @mask: gpio pin mask >> + */ >> +static void ls7a_gpio_i2c_set(struct lsdc_i2c * const li2c, int mask, int state) >> +{ >> + unsigned long flags; >> + u8 val; >> + >> + spin_lock_irqsave(&li2c->reglock, flags); > What are you protecting? Doesn't the caller serialize calls to these > functions? > >> + >> + if (state) { >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + } else { >> + val = readb(li2c->dir_reg); >> + val &= ~mask; >> + writeb(val, li2c->dir_reg); >> + >> + val = readb(li2c->dat_reg); >> + if (state) > This condition is never true. We're in the 'else' because !state. > >> + val |= mask; >> + else >> + val &= ~mask; >> + writeb(val, li2c->dat_reg); > Shouldn't you set the data register low first and then change the > direction? Otherwise, you may be driving high for a moment. However, if > high is always done by setting the direction as input, Setting the direction as input will get high, because the external pull-up resistor will pull the level up. > why write the > data register each time? I'm assuming whatever is written to the dat_reg > is maintained regardless of pin state. > Because we have only one data reg corresponding two direction(input and output). our DC hardware design engineer is not good. :( Value written to the dat_reg will get flushed when pin state changed. Changed from output to input, for example. Otherwise, how can we read the pin level state back? >> + } >> + >> + spin_unlock_irqrestore(&li2c->reglock, flags); >> +} >> + >> +/* >> + * ls7a_gpio_i2c_get - read value back from gpio pin >> + * @mask: gpio pin mask >> + */ >> +static int ls7a_gpio_i2c_get(struct lsdc_i2c * const li2c, int mask) >> +{ >> + unsigned long flags; >> + u8 val; >> + >> + spin_lock_irqsave(&li2c->reglock, flags); >> + >> + /* first set this pin as input */ >> + val = readb(li2c->dir_reg); >> + val |= mask; >> + writeb(val, li2c->dir_reg); >> + >> + /* then get level state from this pin */ >> + val = readb(li2c->dat_reg); >> + >> + spin_unlock_irqrestore(&li2c->reglock, flags); >> + >> + return (val & mask) ? 1 : 0; >> +} >> + >> +/* set the state on the i2c->sda pin */ >> +static void ls7a_i2c_set_sda(void *i2c, int state) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_set(li2c, li2c->sda, state); >> +} >> + >> +/* set the state on the i2c->scl pin */ >> +static void ls7a_i2c_set_scl(void *i2c, int state) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_set(li2c, li2c->scl, state); >> +} >> + >> +/* read the value from the i2c->sda pin */ >> +static int ls7a_i2c_get_sda(void *i2c) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_get(li2c, li2c->sda); >> +} >> + >> +/* read the value from the i2c->scl pin */ >> +static int ls7a_i2c_get_scl(void *i2c) >> +{ >> + struct lsdc_i2c * const li2c = (struct lsdc_i2c *)i2c; >> + >> + return ls7a_gpio_i2c_get(li2c, li2c->scl); >> +} >> + >> +/* >> + * mainly for dc in ls7a1000 which have builtin gpio emulated i2c >> + * >> + * @index : output channel index, 0 for DVO0, 1 for DVO1 >> + */ >> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, void *base, unsigned int index) >> +{ >> + char compat[32] = {0}; >> + unsigned int udelay = 5; >> + unsigned int timeout = 2200; >> + int nr = -1; >> + struct i2c_adapter *adapter; >> + struct lsdc_i2c *li2c; >> + struct device_node *i2c_np; >> + int ret; >> + >> + li2c = devm_kzalloc(dev, sizeof(*li2c), GFP_KERNEL); >> + if (!li2c) >> + return ERR_PTR(-ENOMEM); >> + >> + li2c->index = index; >> + li2c->dev = dev; >> + >> + if (index == 0) { >> + li2c->sda = 0x01; >> + li2c->scl = 0x02; >> + } else if (index == 1) { >> + li2c->sda = 0x04; >> + li2c->scl = 0x08; > Just require this to be in DT rather than having some default. > If only this driver be used in embedded environment(like arms and risc-v), relay on DT solely is the right way. But Loongson display controller have been applied in both PC class desktop(using UEFI+ACPI) and embedded environment(using DT), this is the key difference. I suddenly realize that i didn't explain this fact clearly at that time. The default code works in almost 99% case, through. >> + } >> + >> + spin_lock_init(&li2c->reglock); >> + >> + snprintf(compat, sizeof(compat), "lsdc,i2c-gpio-%d", index); > compatible values shouldn't have an index and you shouldn't need a > index in DT. You need to iterate over child nodes with matching > compatible. > >> + i2c_np = of_find_compatible_node(dev->of_node, NULL, compat); >> + if (i2c_np) { >> + u32 sda, scl; >> + >> + dev_dbg(dev, "Has %s property in the DT", compat); >> + >> + /* */ >> + ret = of_property_read_u32(i2c_np, "sda", &sda); > Custom properties need a vendor prefix. > >> + if (ret == 0) >> + li2c->sda = 1 << sda; >> + >> + ret = of_property_read_u32(i2c_np, "scl", &scl); >> + if (ret == 0) >> + li2c->scl = 1 << scl; >> + >> + /* Optional properties which made the driver more flexible */ >> + of_property_read_u32(i2c_np, "udelay", &udelay); >> + of_property_read_u32(i2c_np, "timeout", &timeout); > These aren't documented. Do you really need them in DT? > >> + of_property_read_u32(i2c_np, "reg", &nr); >> + } >> + >> + dev_dbg(dev, "%s: sda=%u, scl=%u, nr=%d, udelay=%u, timeout=%u\n", >> + compat, li2c->sda, li2c->scl, nr, udelay, timeout); >> + >> + li2c->reg_base = base; >> + >> + li2c->dir_reg = li2c->reg_base + LS7A_DC_GPIO_DIR_REG; >> + li2c->dat_reg = li2c->reg_base + LS7A_DC_GPIO_DAT_REG; >> + >> + li2c->bit.setsda = ls7a_i2c_set_sda; >> + li2c->bit.setscl = ls7a_i2c_set_scl; >> + li2c->bit.getsda = ls7a_i2c_get_sda; >> + li2c->bit.getscl = ls7a_i2c_get_scl; >> + li2c->bit.udelay = udelay; >> + li2c->bit.timeout = usecs_to_jiffies(timeout); >> + li2c->bit.data = li2c; >> + >> + adapter = &li2c->adapter; >> + adapter->algo_data = &li2c->bit; >> + adapter->owner = THIS_MODULE; >> + adapter->class = I2C_CLASS_DDC; >> + adapter->dev.parent = dev; >> + adapter->nr = nr; >> + if (i2c_np) { >> + adapter->dev.of_node = i2c_np; >> + of_node_put(i2c_np); >> + } >> + >> + strscpy(adapter->name, &compat[5], sizeof(adapter->name)); >> + >> + i2c_set_adapdata(adapter, li2c); >> + >> + ret = i2c_bit_add_numbered_bus(adapter); > Why do you care what the bus number is? You shouldn't need to. Yes, we didn't need that. the i2c core will allocate one for us automatically. Your advice is valuable, thanks your kindness review. >> + if (ret) { >> + if (i2c_np) >> + of_node_put(i2c_np); >> + >> + devm_kfree(dev, li2c); >> + return ERR_PTR(ret); >> + } >> + >> + return li2c; >> +} >> + >> +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c) >> +{ >> + struct i2c_adapter *adapter; >> + >> + if (li2c) { >> + adapter = &li2c->adapter; >> + >> + if (adapter && adapter->dev.of_node) >> + of_node_put(adapter->dev.of_node); >> + >> + devm_kfree(ddev->dev, li2c); >> + } >> +} >> + >> +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, >> + unsigned int index) >> +{ >> + const struct lsdc_chip_desc * const descp = ldev->desc; >> + struct lsdc_i2c *li2c; >> + >> + if (index >= descp->num_of_crtc) { >> + drm_err(ldev->ddev, "I2c adapter is no more than %u, %u\n", >> + descp->num_of_crtc, index); >> + return NULL; >> + } >> + >> + li2c = ldev->li2c[index]; >> + if (li2c) >> + return &li2c->adapter; >> + >> + return NULL; >> +} >> diff --git a/drivers/gpu/drm/lsdc/lsdc_i2c.h b/drivers/gpu/drm/lsdc/lsdc_i2c.h >> new file mode 100644 >> index 000000000000..4ab825143eb4 >> --- /dev/null >> +++ b/drivers/gpu/drm/lsdc/lsdc_i2c.h >> @@ -0,0 +1,42 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * KMS driver for Loongson display controller >> + * Copyright (C) 2022 Loongson Corporation >> + */ >> + >> +/* >> + * Authors: >> + * Sui Jingfeng <suijingfeng@loongson.cn> >> + */ >> + >> +#ifndef __LSDC_I2C__ >> +#define __LSDC_I2C__ >> + >> +#include <linux/i2c.h> >> +#include <linux/i2c-algo-bit.h> >> +#include <linux/pci.h> >> + >> +struct lsdc_i2c { >> + struct device *dev; >> + struct i2c_adapter adapter; >> + struct i2c_algo_bit_data bit; >> + /* @reglock: protects concurrent register access */ >> + spinlock_t reglock; >> + void __iomem *reg_base; >> + void __iomem *dir_reg; >> + void __iomem *dat_reg; >> + int index; >> + /* pin bit mask */ >> + u8 sda; >> + u8 scl; >> +}; >> + >> +void lsdc_destroy_i2c(struct drm_device *ddev, struct lsdc_i2c *li2c); >> + >> +struct lsdc_i2c *lsdc_create_i2c_chan(struct device *dev, >> + void *base, >> + unsigned int index); >> + >> +struct i2c_adapter *lsdc_get_i2c_adapter(struct lsdc_device *ldev, >> + unsigned int index); >> +#endif
There is a display controller in loongson's LS2K1000 SoC and LS7A1000 bridge chip, the display controller is a PCI device in those chips. It has two display pipes but with only one hardware cursor. Each way has a DVO interface which provide RGB888 signals, vertical & horizontal synchronisations, data enable and the pixel clock. Each CRTC is able to scanout from 1920x1080 resolution at 60Hz, the maxmium resolution is 2048x2048 according to the hardware spec. Loongson display controllers are simple which require scanout buffers to be physically contiguous. For LS7A1000 bridge chip, the DC is equipped with a dedicated video RAM which is typically 64MB or more. In this case, VRAM helper based driver is suppose to be used. While LS2K1000 is a SoC, only system memory is available, therefore CMA helper based driver is intend to be used. It is possible to use VRAM helper based solution by carving out part of system memory as VRAM though. For LS7A1000, there are 4 dedicated GPIOs whose control register is located at the DC register space, They are used to emulate two way i2c. One for DVO0, another for DVO1. LS2K1000 and LS2K0500 SoC don't have such GPIO hardwared, they grab i2c adapter from other module, either general purpose GPIO emulated i2c or hardware i2c adapter. +------+ +-----------------------------------+ | DDR4 | | +-------------------+ | +------+ | | PCIe Root complex | LS7A1000 | || MC0 | +--++---------++----+ | +----------+ HT 3.0 | || || | | LS3A4000 |<-------->| +---++---+ +--++--+ +---------+ +------+ | CPU |<-------->| | GC1000 | | LSDC |<-->| DDR3 MC |<->| VRAM | +----------+ | +--------+ +-+--+-+ +---------+ +------+ || MC1 +---------------|--|----------------+ +------+ | | | DDR4 | +-------+ DVO0 | | DVO1 +------+ +------+ VGA <--|ADV7125|<--------+ +-------->|TFP410|--> DVI/HDMI +-------+ +------+ The above picture give a simple usage of LS7A1000, note that the encoder is not necessary adv7125 or tfp410, other candicates can be ch7034b, sil9022, ite66121 and lt8618 etc. v2: Fixup warnings reported by kernel test robot v3: Fix more grammar mistakes in Kconfig reported by Randy Dunlap and give more details about lsdc. v4: 1) Add dts required and explain why device tree is required. 2) Give more description about lsdc and VRAM helper based driver. 3) Fix warnings reported by kernel test robot. 4) Introduce stride_alignment member into struct lsdc_chip_desc, the stride alignment is 256 bytes for ls7a1000, ls2k1000 and ls2k0500. v5: 1) Using writel and readl replace writeq and readq, to fix kernel test robot report build error on other archtecture. 2) Set default fb format to XRGB8888 at crtc reset time. v6: 1) Explain why we are not switch to drm dridge subsystem on ls2k1000. 2) Explain why tiny drm driver is not suitable for us. 3) Give a short description of the trival dirty uppdate implement based on CMA helper. v7: 1) Remove select I2C_GPIO and I2C_LS2X in Kconfig, it is not ready now 2) Licensing issues are fixed suggested by Krzysztof Kozlowski. 3) Remove lsdc_pixpll_print(), part of it move to debugfs. 4) Set prefer_shadow to true if vram based driver is in using. 5) Replace double blank lines with single line in all files. 6) Verbose cmd line parameter is replaced with drm_dbg() 7) All warnnings reported by ./scripts/checkpatch.pl --strict are fixed 8) Get edid from dtb support is removed as suggested by Maxime Ripard 9) Fix typos and various improvement v8: 1) Drop damage update implement and its command line. 2) Drop DRM_LSDC_VRAM_DRIVER config option as suggested by Maxime. 3) Deduce DC's identification from its compatible property. 4) Drop the board specific dts patch. 5) Add documention about the display controller device node. v9: 1) Fix the warnings reported by checkpatch script and fix typos v10: 1) Pass `make dt_binding_check` validation 2) Fix warnings reported by kernel test robot v11: 1) Convert the driver to use drm bridge and of graph framework. 2) Dump register value support through debugfs. Below is a brief introduction of loongson's CPU, bridge chip and SoC. LS2K1000 is a double core 1.0Ghz mips64r2 compatible SoC[1]. LS7A1000 is a bridge chip made by Loongson corporation which act as north and/or south bridge of loongson's desktop and server level processor. It is equivalent to AMD RS780E+SB710 or something like that. More details can be read from its user manual[2]. This bridge chip is typically use with LS3A3000, LS3A4000 and LS3A5000 cpu. LS3A3000 is 4 core 1.45gHz mips64r2 compatible cpu. LS3A4000 is 4 core 1.8gHz mips64r5 compatible cpu[3]. LS3A5000 is 4 core 2.5gHz loongarch cpu[4]. Nearly all loongson cpu has the hardware maintain the cache coherency, except for early version of ls2k1000 or ls3a2000. This is the most distinct feature from other Mips cpu. [1] https://wiki.debian.org/InstallingDebianOn/Lemote/Loongson2K1000 [2] https://loongson.github.io/LoongArch-Documentation/Loongson-7A1000-usermanual-EN.html [3] https://ee-paper.com/loongson-3a4000-3b4000-motherboard-products-are-compatible-with-uos-system/ [4] https://loongson.github.io/LoongArch-Documentation/Loongson-3A5000-usermanual-EN.html [5] https://github.com/loongson-community/pmon suijingfeng (7): MIPS: Loongson64: dts: update the display controller device node MIPS: Loongson64: dts: introduce ls3A4000 evaluation board MIPS: Loongson64: dts: introduce lemote A1901 motherboard MIPS: Loongson64: dts: introduce ls2k1000 pai evaluation board dt-bindings: display: Add Loongson display controller MIPS: Loongson64: defconfig: enable display bridge drivers on Loongson64 drm/lsdc: add drm driver for loongson display controller .../loongson/loongson,display-controller.yaml | 230 +++++++ arch/mips/boot/dts/loongson/lemote_a1901.dts | 92 +++ .../boot/dts/loongson/loongson64-2k1000.dtsi | 24 + arch/mips/boot/dts/loongson/ls2k1000_pai.dts | 102 ++++ .../boot/dts/loongson/ls3a4000_7a1000_evb.dts | 136 +++++ arch/mips/boot/dts/loongson/ls7a-pch.dtsi | 36 +- arch/mips/configs/loongson2k_defconfig | 5 + arch/mips/configs/loongson3_defconfig | 5 + drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/lsdc/Kconfig | 23 + drivers/gpu/drm/lsdc/Makefile | 13 + drivers/gpu/drm/lsdc/lsdc_crtc.c | 396 ++++++++++++ drivers/gpu/drm/lsdc/lsdc_drv.c | 547 +++++++++++++++++ drivers/gpu/drm/lsdc/lsdc_drv.h | 197 ++++++ drivers/gpu/drm/lsdc/lsdc_i2c.c | 235 +++++++ drivers/gpu/drm/lsdc/lsdc_i2c.h | 42 ++ drivers/gpu/drm/lsdc/lsdc_irq.c | 58 ++ drivers/gpu/drm/lsdc/lsdc_irq.h | 18 + drivers/gpu/drm/lsdc/lsdc_output.c | 262 ++++++++ drivers/gpu/drm/lsdc/lsdc_output.h | 24 + drivers/gpu/drm/lsdc/lsdc_pci_drv.c | 328 ++++++++++ drivers/gpu/drm/lsdc/lsdc_plane.c | 470 ++++++++++++++ drivers/gpu/drm/lsdc/lsdc_pll.c | 574 ++++++++++++++++++ drivers/gpu/drm/lsdc/lsdc_pll.h | 88 +++ drivers/gpu/drm/lsdc/lsdc_regs.h | 220 +++++++ 26 files changed, 4123 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/loongson/loongson,display-controller.yaml create mode 100644 arch/mips/boot/dts/loongson/lemote_a1901.dts create mode 100644 arch/mips/boot/dts/loongson/ls2k1000_pai.dts create mode 100644 arch/mips/boot/dts/loongson/ls3a4000_7a1000_evb.dts create mode 100644 drivers/gpu/drm/lsdc/Kconfig create mode 100644 drivers/gpu/drm/lsdc/Makefile create mode 100644 drivers/gpu/drm/lsdc/lsdc_crtc.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_drv.h create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_i2c.h create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_irq.h create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_output.h create mode 100644 drivers/gpu/drm/lsdc/lsdc_pci_drv.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_plane.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.c create mode 100644 drivers/gpu/drm/lsdc/lsdc_pll.h create mode 100644 drivers/gpu/drm/lsdc/lsdc_regs.h