Hi Linus, On Tue, Dec 03, 2019 at 04:20:24PM +0100, Linus Walleij wrote: > On Tue, Dec 3, 2019 at 8:47 AM Maxime Ripard <mripard@xxxxxxxxxx> wrote: > > > Using only the mode as we do currently has a bunch of shortcomings as > > almost no encoder will be able to provide the typical pixel clock, and > > that situation leads to multiple things: > > > > - If someone working on one encoder wants to upstream a panel they > > have tested, chances are this will not be the typical pixel clock > > / timings being used but rather the one that will match what that > > SoC is capable of. Trouble comes when a second user comes in with > > a different encoder and different capabilities, and then we have a > > maintainance fight over which timing is the true timing (with a > > significant chance that none of them are). > > > > - If we can't match the pixel clock, we currently have no easy way > > to make the usual measures of reducing / growing the porches and > > blankings areas to match the pixel clock we can provide, since we > > don't have an easy way to get the tolerance on those timings for a > > given panel. There's some ad hoc solutions on some drivers (I > > think vc4 has that?) to ignore the panel and just play around with > > the timings, but I think this should be generalised. > > I've been confused with these things as they look today and it seems > the whole struct drm_display_mode could need some improvement? > > If .clock is supposed to be htotal * vtotal * vrefresh, what is the > .clock doing there anyway. It's one thing I wonder as well. I guess it's just more convenient for everyone, since it's exposed by the VESA modes (iirc) and a lot of drivers really care about the clock. > Sadly I am too inexperienced to realize where the tolerances should > be stated, but I guess just stating that hsync_start etc are typical, > then specify some tolerance for each would help a bit? The timings structure discussed in the patch that started this discussion is actually doing this nicely, you have for each timing the minimum, typical and maximum value. The current issue with it though is that it's pretty difficult to use it, since it's not really tied to any of the panel code (or DRM helpers). The only driver that was supporting it was omapdrm and it was removed recently. If we really wanted to support it, one path forward I can see would be to make the timings structure the primary one, and only use drm_display_mode for userspace facing code, and generate it from the timings. This would be a pretty invasive change though... > On the DSI displays in video mode there is also this EOL area > which seems to be where the logic is normally just idling for a > while, that can be adjusted on some hardware as well, but > I don't quite understand it admittedly. Sometimes I wonder if > anyone really understands DSI... :/ I'm not aware of any EOL area in MIPI-DSI that would make the hardware idle, don't you mean LP-11? Maxime
Attachment:
signature.asc
Description: PGP signature
