raspberrypi / firmware

This repository contains pre-compiled binaries of the current Raspberry Pi kernel and modules, userspace libraries, and bootloader/GPU firmware.
5.15k stars 1.68k forks source link

DSI display resolution seems to be hardcoded #1819

Open arturo182 opened 1 year ago

arturo182 commented 1 year ago

I'm using the CM4 with a custom DSI display. I have it working under Linux by using an overlay and a custom entry in the simple-panel.c file. However on boot, before the kernel takes over the DSI and it's still the bootloader driving it, the initial rainbow square and some of the boot log lines are all scrambled because the resolution of my display is not 1280x720 like the official display. Is there a way to tell the bootloader to use a different size for the framebuffer, the framebuffer_width and framebuffer_height values do not seem to change anything on DSI. I would love it if there was a way to do it because right now my device displays a jumbled-up mess on the display for the first 15 seconds after power-on and it's just not great for the users.

6by9 commented 1 year ago

The Pi 7" panel is 800x480. That is the only panel that the firmware knows how to drive. That's not going to change, and is why we've invested the time to get the vc4-kms-v3d driver working with DSI panels.

The firmware should only start up DSI if it finds what it believes to be a Pi 7" panel connected. Making a custom panel that pretended to be a Pi 7" 800x480 display when it isn't would be a weird design decision. More normal would be to have the relevant dtoverlay= line in config.txt, and wait until vc4 initialises. If you need the display to come up sooner, then you can alter the defconfig to build vc4 and dependencies into the main kernel image, and then the display is up around 1.5s after Linux starts.

6by9 commented 1 year ago

And framebuffer_width and framebuffer_height have only ever changed the geometry of the plane being used for the framebuffer, never the output mode being used to drive the display.

arturo182 commented 1 year ago

Right, sorry, I got confused about the Pi 7" panels' resolution. I was hoping I could get the simple-panel.c addition merged into the RPi kernel and that way I could get away with having the stock kernel, and only require a custom overlay and config.txt changes. Could you expand a bit about building vc4 into the kernel, do you mean change it from being a module to "yes" or is there more to it?

6by9 commented 1 year ago

We're open to merging support for other panels. We already have Pimoroni HyperPixel DPI panels, the CutiePi ILI9881, JDI LT070ME05000 1200x1920, and Waveshare's DSI panels supported and with overlays in the kernel tree. That is the kernel though, not the firmware. The rainbow square and early boot screens therefore aren't supported.

There is a debate over whether panel timings should be in DT (as we've done with the HyperPixel panels), or with a panel-simple entry. A compatible string and panel-simple entry has the benefit that if you later find out the timing is wrong, at least updating the driver will get all users of it, whereas if it's in DT then you've potentially got a bigger issue fixing up random overlays.

Yes I'm meaning changing from being a module to being builtin as "yes". As a module the display will typically be initialised about 7-8seconds after Linux starts as the file system is needed for /lib/modules. The downside is that there are a large number of dependencies to vc4 (eg chunks of SND to support HDMI audio), so it's not a simple one module that needs to be switched over. Doing so will also increase the size of the kernel image, which is one of the main reasons it isn't built in by default as it would be a significant negative effect to headless users.

timonsku commented 1 year ago

I'm currently working on product for Adafruit that would support different panels via a bridge IC like it is used in the official touchscreen. For very simple DSI devices like bridges (DSI to HDMI is also common) or simple DSI panels that are driven by the panel-dsi or panel-simple driver like @arturo182 is using, would an option to set a display mode/timing at runtime be something you could consider @6by9 ? Those panels do not require an init sequence or have that handled externally and only require the correct drm mode to be set or whatever the equivalent in the firmware binary would be.

Most interesting would be support for reading/probing for EDID on the I2C that is present on the DSI connector, that would enable the most use cases without needing to provide specific resolutions to any given system in software or needing to add lots of panel timing definitions to the firmware and the kernel (this is already supported on the Kernel side by panel-simple) DSI to HDMI/DP bridges would also profit directly from that and adding an EDID EEPROM is very cost efficient for embedded applications where you bring a specific panel. Given you support EDID already for the bootloader over HDMI I was hoping that this could be a fairly straight forward solution to be implement on the DSI end as well.

Having the option to define such a timing in config.txt akin to the HDMI timing option would be a nice fallback though.

6by9 commented 1 year ago

There will be no changes to the firmware for driving DSI. All additional DSI devices will only be supported via Linux kernel drivers.

Some DSI to HDMI bridges already have kernel drivers, but they are generally complex devices. Reading EDIDs for these is generally taken care of within these devices anyway, and don't need a directly attached EDID EEPROM. ADV7511 and ADV7533 both require the DSI rate to exactly match the HDMI pixel rate, and that is not achievable on the vc4 DSI block. Based on the number of messages on dri-devel the Lontium chips are a bit of a pain generally as well.

EDIDs don't really match with DSI panels as they don't contain any of the information with regard number of DSI data lanes. Seeing as that is quite so fundamental to DSI, my view is that is a dead end.

What might be considered is looking for a HAT EEPROM over the panel's I2C link so that the relevant overlay can be loaded automatically. It's not something that has been discussed at all let alone planned, and has the downside that a genuine device could be sitting on the 0x50 address that the HAT EEPROM adopts (there's a reason that HATs have a dedicated I2C bus for the EEPROM).

timonsku commented 1 year ago

Could you elaborate what you mean with "All additional DSI devices will only be supported via Linux kernel drivers."? As in there will be no support for DSI devices in the bootloader outside of the official 7" screen resolution?

@ HDMI bridges, sure they don't need a EEPROM on the board that would come from the monitor.

To be clear I'm not interested in supporting DSI panels in the bootloader, solely bridges which only ingest the video stream (or DSI panels that can be initialized externally like most bridges). You are correct that the lane count is something that has no standard way of being defined in EDID but it really is the only thing that truly needs to be set by the host and could definitely live in config.txt (or as a vendor block in EDID). Everything else I as a user can take care of myself in my system and don't need the host to initialize anything.

Even if the bootloader would be constrained to single lane operation that would be tolerable if I could change the timings in any way. EDID would be ellegant as its a standard way of communicating that information and works also as is with existing kernel drivers without modifications.

Having the ability to check for HAT EEPROMs on the DSI I2C would be great. A solution to a different issue than the bootloader but also something that would make using new display solutions much easier both for large scale industry deployment and more maker/prototype use cases. I would see the risks there as low as its on a specific set of pins that are not accessible to users who are not designing their own PCBs specifically for this use case. As a hardware designer it would be no problem to work around that probe behaviour if I really have an address conflict, generally the chances for that are low though in my experience I2C devices tend to avoid that standard EEPROM address. I can totally see why the choice was made for the 40pin header but I think in this location the benefits far outweigh the (slim) cons.

6by9 commented 1 year ago

Could you elaborate what you mean with "All additional DSI devices will only be supported via Linux kernel drivers."? As in there will be no support for DSI devices in the bootloader outside of the official 7" screen resolution?

Correct. The firmware will ever only support the official 7" 800x480 screen. Clones of the 7" screen take their chances as the drivers are written for the Pi screen. If issues should be found with the Pi panel, then those drivers will change. There have already been issues with the DFRobot screen because they chose to emulate part of the FT5x06 touch controller because theirs was something different, so the switch to the kernel drivers failed because DFRobot hadn't emulated the way that the kernel driver read from the controller. Sorry, that is not Raspberry Pi's problem.

Seeing as everything else except reading the HAT EEPROM comes from the Linux kernel, you're free to raise PRs for anything proposed.

My personal opinion currently is that EDIDs are a very bad fit for butchering in DSI support. I'd need to see a very clean implementation to consider adding the support for it.

panel-simple.c already supports the compatible string of panel-dpi and a panel-timing section in DT. Adding an equivalent panel-dsi for DSI is feasible, reading the panel-timing from DT in the same way, and a data-lanes property in the endpoint to relay the number of lanes in the same way that V4L2 does for CSI2. That still leaves an unanswered question over how the other DSI flags for BURST, SYNC_PULSE, etc, would be relayed in DT. Pixel format can be assumed to be RGB888, or could be added as a bus-format property in the same way as for panel-dpi.

arturo182 commented 1 year ago

While researching this before, I have ran into the Rockchip kernel and found that it has support for specifying DSI timings, lanes, and flags in the dts: https://github.com/Supernote-Ratta/kernel_a6x_a5x/blob/bdc77c244cd3d6318da3b1e8503dde087aadac08/arch/arm64/boot/dts/rockchip/super78.dts#L529

This is of course a custom change made by them: https://github.com/Supernote-Ratta/kernel_a6x_a5x/commit/f6972eb49466611c9afc5cdad706813143e70fa6

6by9 commented 1 year ago

AIUI That isn't the main Rockchip vendor kernel - see https://github.com/rockchip-linux/kernel for their actual kernel release. If you wish to stick on 4.4, 4.19, or 5.10 kernel then feel free. Raspberry Pi are generally on the latest LTS release, therefore we also try and minimise the number of downstream patches that are required, upstreaming patches where appropriate.

Dropping display timings into DT has come up before. Find that patch series as it was submitted to dri-devel, and you can read the response of mainline DT maintainers to it. https://patchwork.freedesktop.org/patch/99876/ https://patchwork.freedesktop.org/patch/99875/

Sorry, not going to happen. Read this for an explanation of why not: https://sietch-tagr.blogspot.de/2016/04/display-panels-are-not-special.html Thierry

When you get such a strong pushback on a proposal, you know you're flogging a dead horse.

Look at Rockchip's main kernel branch and you'll find many more hacks to panel-simple.c that won't be upstreamable - https://github.com/rockchip-linux/kernel/blob/develop-4.4/drivers/gpu/drm/panel/panel-simple.c

I'm actually slightly surprised that https://github.com/torvalds/linux/commit/4a1d0dbc8332231d1d500d7a1d13c45457262a97 got merged to add panel timings via panel-dpi, but there you go. If DT accepted it for panel-dpi then they can have limited complaints about doing exactly the same thing on dsi, however both have limitations in the mainline kernel.

timonsku commented 1 year ago

I'm actually slightly surprised that torvalds/linux@4a1d0db got merged to add panel timings via panel-dpi, but there you go. If DT accepted it for panel-dpi then they can have limited complaints about doing exactly the same thing on dsi, however both have limitations in the mainline kernel.

Oh that is really interesting! I'm aware of the long and heated discussion regarding timings in DT. This is the reason I went the EDID route so far as it seemed like the only accepted way so far to define timings at run-time without kernel boot flag hacks.

I agree with you that its not an optimal route for DSI but it seemed like the most acceptable and maintainable one from both kernel and product use perspective, so far at least.

In light of those upstream changes, would you be open to changes to panel-simple that are similar to the more recent panel_dpi additions but for DSI? I think this would be an elegant solution and given the precedent also seems safe like you said to eventually commit upstream with the necessary primitives already present. I have the resources to work on a proper solution here at the moment, would you be open to a PR of a backport of panel-simple from current main that would include an implementation for a panel-dsi?

Regarding the bootloader situation, I definitely understand not wanting to provide guarantees to compatibility to hacks happening so far with DSI panels to utilize the Pi drivers. Although it is tempting as its really the only way at the moment to get any functionality in that (very useful!) early boot environment. I'm guessing not wanting to do any changes on the bootloader side is also related to the reason the firmware needs to remain closed source? If that is better to discuss in an NDA friendly environment am also happy to talk about this via email (timon [at] diodes-delight.com) But if there is nothing that can be done on that front I also understand!

timonsku commented 1 year ago

Oh and yea support for HAT EEPROM on DSI I2C would be fantastic for that, that could give an equivalent or rather better experience for hardware side defined configuration rather than only timings from something like EDID. If that is something you can see as being feasible to be added to a future firmware update I would be happy to assist with any alpha testing and would have hardware that could immediately make use of such a feature.

6by9 commented 1 year ago

In light of those upstream changes, would you be open to changes to panel-simple that are similar to the more recent panel_dpi additions but for DSI? I think this would be an elegant solution and given the precedent also seems safe like you said to eventually commit upstream with the necessary primitives already present. I have the resources to work on a proper solution here at the moment, would you be open to a PR of a backport of panel-simple from current main that would include an implementation for a panel-dsi?

Yes I would be open to adding a compatible panel-dsi, which uses a panel-timings DT entry to set up the DSI timings. Please ensure the patch follows the kernel coding standards, and that they have appropriate commit messages and Signed-off-by tags because we probably would be looking to upstream them.

Re HAT EEPROM read on DSI I2C, that has the issue that the same I2C bus is used for both camera and display, and it would work fine if either had a suitable HAT EEPROM. If you attach both a camera and display and both have HAT EPROMs fitted, then it falls down as neither can be read reliably. You could define that displays should use 0x50 and cameras 0x52 or similar (0x51 is used by the PoE HAT so needs to be avoided), but it just gets more and more complex.

timonsku commented 1 year ago

@ driver, awesome, that would be my plan that it is in a state that is acceptable for upstream as well. Will get started this week and open a draft PR on the Kernel repo for feedback once its in a sensible state.

@ hat eeprom. That is definitely a good point but the solution you mention seems to solve that quite nicely? I would see both of these connectors as "advanced use". Having a little bit of higher implementation complexity by needing to choosing/setting an EEPROM with a specific base address for a port seems very reasonable to me. I think the important bit here might be to use different language to make this clear to implementers, e.g. there is not a "HAT interface" on the DSI and CSI ports but a DSI DT and CSI DT EEPROM depending on which port you wish to utilize. That interface would largely be compatible with what is specified in the HAT format but has those slight changes for this more specific use case. Happy to think through the system here if you foresee further complexity or issues that need to be avoided.

timonsku commented 11 months ago

Draft PR here: https://github.com/raspberrypi/linux/pull/5640 It's working with the devices I have to test here. I think generally it would be ready to be merged but would appreciate any feedback you might have.