Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-04-07 Thread Sui Jingfeng



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.



Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-24 Thread Rob Herring
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(>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/
> 


Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-24 Thread Rob Herring
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 
> > > 
> > > 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", );
> > > + of_property_read_u32(i2c_np, "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


Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-24 Thread Sui Jingfeng



On 2022/3/23 04:49, Rob Herring wrote:

+   }
+
+   spin_lock_init(>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/



Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-23 Thread Sui Jingfeng



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.



Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-23 Thread Sui Jingfeng



On 2022/3/23 04:49, Rob Herring wrote:

On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote:

From: suijingfeng 

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 XRGB 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 
Signed-off-by: suijingfeng 
Signed-off-by: Sui Jingfeng <15330273...@189.cn>
Signed-off-by: suijingfeng 
---
  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 

Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-23 Thread Rob Herring
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


Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-23 Thread Sui Jingfeng



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.



Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-23 Thread Sui Jingfeng



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(>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(>reglock, flags) and 
spin_unlock_irqrestore(>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.



Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-22 Thread Sui Jingfeng



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.




Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-22 Thread Sui Jingfeng



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 = <>;
        };

[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.




Re: [PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-22 Thread Rob Herring
On Tue, Mar 22, 2022 at 12:29:16AM +0800, Sui Jingfeng wrote:
> From: suijingfeng 
> 
> 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 XRGB 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 
> Signed-off-by: suijingfeng 
> Signed-off-by: Sui Jingfeng <15330273...@189.cn>
> Signed-off-by: suijingfeng 
> ---
>  drivers/gpu/drm/Kconfig |   2 +
>  drivers/gpu/drm/Makefile|   1 +
>  drivers/gpu/drm/lsdc/Kconfig|  23 ++
>  drivers/gpu/drm/lsdc/Makefile   |  13 

[PATCH v11 7/7] drm/lsdc: add drm driver for loongson display controller

2022-03-21 Thread Sui Jingfeng
From: suijingfeng 

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 XRGB 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 
Signed-off-by: suijingfeng 
Signed-off-by: Sui Jingfeng <15330273...@189.cn>
Signed-off-by: suijingfeng 
---
 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