On 2022/2/10 00:16, Maxime Ripard wrote:
On Wed, Feb 09, 2022 at 10:38:41PM +0800, Sui Jingfeng wrote:
On 2022/2/9 16:49, Maxime Ripard wrote:
On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
+/* Get the simple EDID data from the device tree
+ * the length must be EDID_LENGTH, since it is simple.
+ *
+ * @np: device node contain edid data
+ * @edid_data: where the edid data to store to
+ */
+static bool lsdc_get_edid_from_dtb(struct device_node *np,
+ unsigned char *edid_data)
+{
+ int length;
+ const void *prop;
+
+ if (np == NULL)
+ return false;
+
+ prop = of_get_property(np, "edid", &length);
+ if (prop && (length == EDID_LENGTH)) {
+ memcpy(edid_data, prop, EDID_LENGTH);
+ return true;
+ }
+
+ return false;
+}
You don't have a device tree binding for that driver, this is something
that is required. And it's not clear to me why you'd want EDID in the
DTB?
1) It is left to the end user of this driver.
The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
which don't have DDC support either, doing this way allow them put a
EDID property into the dc device node in the DTS. Then the entire system works.
Note those panel usually support only one display mode.
I guess it depends on what we mean exactly by the user, but the DTB
usually isn't under the (end) user control. And the drm.edid_firmware is
here already to address exactly this issue.
On the other end, if the board has a static panel without any DDC lines,
then just put the timings in the device tree, there's no need for an
EDID blob.
Loongson have a long history of using PMON firmware, The PMON firmware
support flush the dtb into the the firmware before grub loading the kernel.
You press 'c' key, then the PMON will give you a shell. it is much like a
UEFI shell. Suppose foo.dtb is what you want to pass to the vmlinuz.
Then type the follow single command can flush the dtb into the PMON firmware.
|load_dtb /dev/fs/fat@usb0/foo.dtb|
For our PMON firmware, it**is** totally under developer/pc board maker's control.
You can flush whatever dtb every time you bootup until you satisfied.
It(the pmon firmware) is designed to let downstream motherboard maker and/or
customers to play easily.
Support of reading EDID from the dtb is really a feature which downstream
motherboard maker or customer wanted. They sometimes using eDP also whose
resolution is not 1024x768. This is out of control for a graphic driver
developer like me.
And, to reinstate, we already have a mechanism to set an EDID, and if it
wasn't an option, the DT is not the place to store an EDID blob.
I know, put edid blob in the dts maybe abuse, but i am not push dts with edid blob either.
It is left to other people, and the ./arch/powerpc/boot/dts/ac14xx.dts already have edid blob.
And drm.edid_firmware have only a few limited resolution which is weak.
You're wrong. There's no limitation, it's just as limited as your
solution. You put the same thing, you get the same thing out of it. The
only difference is where the data are coming from.
It is extremely difficult to use, it have difficulty to specify which firmware edid is for which connector.
because we have a 1024x600 panel and a 1920x1080 monitor.
It require you to know the connector's name at first, it is not as intuitive as my method.
I am exhausted by it.
I will consider to adding drm.edid_firmware support, thanks.
It just works if you use drm_get_edid.
2) That is for the display controller in ls2k1000 SoC.
Currently, the upstream kernel still don't have GPIO, PWM and I2C driver support
for LS2K1000 SoC.
How dose you read EDID from the monitor without a I2C driver?
without reading EDID the device tree support, the screen just black,
the lsdc driver just stall. With reading EDID from device tree support
we do not need a i2c driver to light up the monitor.
This make lsdc drm driver work on various ls2k1000 development board
before I2C driver and GPIO driver and PWM backlight driver is upstream.
I have many local private dts with the bindings, those local change just can not
upstream at this time, below is an example.
The device tree is a platform description language. It's there to let
the OS know what the hardware is, but the state of hardware support in
the said OS isn't a parameter we have to take into account for a new
binding.
If you don't have any DDC support at the moment, use the firmware
mechanism above, or add fixed modes using drm_add_modes_noedid in the
driver, and leave the DT out of it. Once you'll gain support for the
EDID readout in the driver, then it'll just work and you won't need to
change the DT again.
The resolution will be 1024x768, it will also add a lot modes which may
not supported by the specific panel. Take 1024x600 as an example,
Both drm_add_modes_noedid() and firmware mechanism above will fail.
Because the user supply EDID only and manufacturer of some strange panel
supply EDID only.
It's fairly easy to address: if the panel has some EDID, make the driver
able to read it; if it doesn't, describe the mode in the DT.
And if you want to be nice to your users, the firmware can even patch
the DT at boot time to add the necessary bits based on whatever info it
has, it doesn't have to be static.
Maxime
