On Wed, Dec 19, 2018 at 7:35 PM Daniel Vetter <daniel@xxxxxxxx> wrote: > > On Wed, Dec 19, 2018 at 6:14 PM Lubomir Rintel <lkundrak@xxxxx> wrote: > > > > On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote: > > > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@xxxxx> wrote: > > > > Hi, > > > > > > > > here are patches that make the Armada DRM work on an OLPC XO-1.75. > > > > They build on patches previously submitted by Russel King (included here for > > > > completeness as well). > > > > > > > > They certainly need some more love, but I'm not able to improve them until > > > > I get to understand some things first. I'm posting a couple of questions > > > > below in hope someone is kind enough to help me deal with my confusion. > > > > > > > > The display pipeline on the laptop looks like this: > > > > > > > > Armada LCDC > > > > ----------- > > > > | > > > > v > > > > > > > > Himax HX8837 "DCON" > > > > ------------------- > > > > RGB input from LCDC > > > > Controlled via I2C > > > > Backlight control > > > > Can slow down the panel refresh rate to save power > > > > Optional dithering for color mode, "swizzling" > > > > Has DRAM attached, can freeze a single frame in it > > > > Doc: http://wiki.laptop.org/images/0/09/DCON_datasheet_HX8837-A.pdf > > > > | > > > > v > > > > > > > > Innolux LS075AT011 Panel > > > > ------------------------ > > > > 7.5", 1200x900 > > > > No datasheet. > > > > http://wiki.laptop.org/go/Display > > > > Can opreate in two modes: > > > > > > > > R G B > > > > G B R ... or "e-book" daylight readable > > > > B R G reflexive B&W > > > > . > > > > : > > > > > > > > Here's what's not clear to me: > > > > > > > > 1.) Is the Himax chip an encoder here? > > > > > > Since it's an external IP probably better to model it as a bridge. > > > > > > > 2.) What's the use of an encoder anyways? If a panel was connected directly > > > > do the RGB output from the LCDC, would we have to fake one? Is that the > > > > point of things like i.MX driver's drivers/gpu/drm/imx/parallel-display.c? > > > > > > encoder is the thing between crtc and connector. It's exposed to > > > userspace (which is a uapi design accident, but can't change that), > > > hence why it's required and some drivers have dummy ones. > > > > > > The real usage is purely internally for the atomic helpers. > > > > I'm wondering how should the drm_encoder be instantiated? The imx uses > > a dummy (compatible="fsl,imx-parallel-display") node. That almost > > certainly counts as devicetree abuse and is a wrong thing to do. > > Should the armada driver always create one? In that case the > > transcoders that connect to it (just tda998x, I suppose) should cease > > doing so and hook on a bridge instead. > > Not a dt expert, but on all your other questions's I'd say yes. Note > that if there's any place where a 100% dummy drm_encoder blows up we > should change that. Anything beyond drm_encoder_init shouldn't be > necessary. > > > Or should the driver for the Himax chip create an encoder and bridge > > intance? Perhaps not, if my understanding is correct that drm_encoder > > shall not be used for things outside the lcdc/crtc chip. > > drm_bridge is the interface for external transcoder IP, not > drm_encoder. As mentioned, only reason tda998x creates an encoder in > some cases is for historical reasons. kms overview might also help with navigating the big picture: https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html (I just noticed a misplace link target, patch on its way). -Daniel > > > > > Most > > > callbacks in the encoder should be optional, so a dummy encoder should > > > be a lot thinner than your imx example. The imx example should be > > > using the panel-bridge I think, which would take care of most of the > > > boilerplate (but panel-bridge didn't exist back then I guess). > > > > > tda998x predates bridges, which is why there's still some > > > encoder_helper usage in there. It's become a proper bridge driver now > > > I think (but some drivers still use the old interface). All new > > > transcoder chip drivers should be bridges. > > > > Which drivers are good examples? > > Anything under drm/bridge should be fine. > > > > Did you look at the > > > kerneldoc for bridges? If those aren't clear we need to fix them. > > > > Yes, if you mean this: > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#bridges > > > > If they weren't clear it might be entirely my fault. I'm totally new to > > most of the stuff there and the sheer size of the structures and the > > API is a tough challenge for my working memory. > > > > The two important things that were not clear to me are essentially > > what's discussed above. That is: > > > > 1.) Should I use this at all? > > From "These are handy when a regular drm_encoder entity isn’t enough to > > represent the entire encoder chain." I wrongly concluded that I > > probably don't heed to. > > Hm yeah I guess that should mention a postive example like > - external transcoder block > - if you want to share transcoder code between different drivers. > > Care to write a patch to improve the wording? > > > 2.) Which driver creates the drm_encoder object and which one connects > > it to the drm_bridge. > > Main driver needs to create the drm_encoder, shared transcoder driver > only creates the bridge. > > Again, care to clarify this in a patch? Might be also good to add > links to the various functions for registering/finding a bridge and > how to link it up to the encoder, and who exactly should do that. > > > > > 4.) How shall I expose the fancy functionality of the Himax to the userspace? > > > > Notably, the freeze frame. The OLPC laptops with the stock firmware like > > > > to suspend the SoC very aggressively to save power (in 20 seconds of > > > > inactivity or so). If the display is open (it can also be turned around > > > > for a tablet or e-book mode), it makes sense to freeze the picture and > > > > keep the panel on, if the laptop is closed, we want to turn it off. > > > > Should the behavior be exposed via sysfs (as it is with the current OLPC > > > > kernels), or a DRM property? Would it require libdrm support? > > > > > > I've accidentally seen how that's implemented for fbdev in the older > > > olpc drivers, and that's definitely _not_ how we want to support this. > > > Essentially what you have here is a fancy self-refresh panel. That's > > > already fully supported by DRM uapi, so I'd say first implement that. > > > Once we have that we can figure out a special property that tells the > > > driver to not shut down the screen on suspend, but just go into > > > self-refresh. Implementing the self-refresh stuff in DRM will be the > > > big pile of work (since that's something which goes beyond the generic > > > drm_bridge interface). > > > > Thank you, this looks pretty helpful. > > > > Currently the MMP2 platform barely boots with the mainline kernel, let > > alone suspends and resumes. I guess it would be all right if the self- > > refresh-on-suspend trick gets worked out later, when the OLPC patch > > queue drains a bit. > > > > How do the DRM properties get set? Do they need support in some > > userspace library, perhaps libdrm? > > You'll need to write the compositor patches in some open source > userspace to demonstrate the full stack, see: > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements > > I guess you're still very far away from that point. libdrm itself > shouldn't need any changes. > > > > > Himax HX8837 "DCON" > > > > ------------------- > > > > RGB input from LCDC > > > > Controlled via I2C > > > > > > These two shouldn't be a problem. > > > > > > > Backlight control > > > > > > We already have backlight interfaces > > > > Yes; this is already supported in the RFC version of the driver chained > > to this message. > > > > > > Can slow down the panel refresh rate to save power > > > > > > Just expose this as modes with different vrefresh (and do that > > > modeswitch without doing a full modeset). Bonus if you do it > > > automatically, which can be done with the self-refresh infrastructure. > > > > > > > Optional dithering for color mode, "swizzling" > > > > > > Not exactly clear, but I'm assuming the bus_format stuff on bridges > > > should help with this. > > > > > > > Has DRAM attached, can freeze a single frame in it > > > > > > Classic self-refresh support. Noralf Tronnes has done great work here > > > past year to improve the infrastructure and let drivers implement this > > > with less typing. Also look at the recent work by vmwgfx folks. > > > > Thanks for above the pointers. > > > > > Cheers, Daniel > > > > I appreciate your response very much. > > Happy to help out, especially if it results in improved documentation. > Often the experts don't see what's missing because too close, and > people newly learning are best suited to spot&fill the gaps. > > > I'll try to digest it over the holidays and hopefully figure out how to > > follow up with an updated version sometime early next year. > > > > If I'm unsuccessful perhaps I'll manage to take some poor graphics > > hacker hostage at FOSDEM. > > I won't be at fosdem, but should be plenty of victims for you there :-) > > Cheers, Daniel > > > > > Take care, > > Lubo > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel