| Message ID | 20181218153742.1313125-1-lkundrak@v3.sk (mailing list archive) |
|---|---|
| Headers | show |
| Series | Armada DRM support for OLPC XO-1.75 laptop | expand |
On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> 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. 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). > 3.) My patch set currently contains the driver for the Himax that is > modelled after tda998x. That one implements an encoder. Similar drivers > seem to add a bridge too, but it's not entirely clear to me what a bridge > is good for? 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. Did you look at the kerneldoc for bridges? If those aren't clear we need to fix them. > 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). > Himax HX8837 "DCON" > ------------------- > RGB input from LCDC > Controlled via I2C These two shouldn't be a problem. > Backlight control We already have backlight interfaces > 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. Cheers, Daniel
On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote: > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> 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. 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. > 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? > 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. 2.) Which driver creates the drm_encoder object and which one connects it to the drm_bridge. > > 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? > > 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. 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. Take care, Lubo
On Wed, Dec 19, 2018 at 6:14 PM Lubomir Rintel <lkundrak@v3.sk> 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@v3.sk> 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. > > > 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 >
On Wed, Dec 19, 2018 at 7:35 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Dec 19, 2018 at 6:14 PM Lubomir Rintel <lkundrak@v3.sk> 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@v3.sk> 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
Hi, On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote: > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> 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. 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). > > > 3.) My patch set currently contains the driver for the Himax that is > > modelled after tda998x. That one implements an encoder. Similar drivers > > seem to add a bridge too, but it's not entirely clear to me what a bridge > > is good for? > > 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. Did you look at the > kerneldoc for bridges? If those aren't clear we need to fix them. > > > 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). > > > > Himax HX8837 "DCON" > > ------------------- > > RGB input from LCDC > > Controlled via I2C > > These two shouldn't be a problem. > > > Backlight control > > We already have backlight interfaces > > > 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. I'm wondering if you can share some more specific pointers? Grepping for "self-refresh" yields references to the eDP self refresh, but that seems not relevant as that one depends on hardware to decide when to enter the self-refresh mode. Perhaps I was not looking at the right place (I searched git and patchwork), but I was not able to find work that would seem relevant. > Cheers, Daniel Thank you, Lubo
On Thu, Jan 03, 2019 at 11:03:36AM +0100, Lubomir Rintel wrote: > Hi, > > On Wed, 2018-12-19 at 10:13 +0100, Daniel Vetter wrote: > > On Wed, Dec 19, 2018 at 9:40 AM Lubomir Rintel <lkundrak@v3.sk> 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. 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). > > > > > 3.) My patch set currently contains the driver for the Himax that is > > > modelled after tda998x. That one implements an encoder. Similar drivers > > > seem to add a bridge too, but it's not entirely clear to me what a bridge > > > is good for? > > > > 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. Did you look at the > > kerneldoc for bridges? If those aren't clear we need to fix them. > > > > > 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). > > > > > > > Himax HX8837 "DCON" > > > ------------------- > > > RGB input from LCDC > > > Controlled via I2C > > > > These two shouldn't be a problem. > > > > > Backlight control > > > > We already have backlight interfaces > > > > > 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. > > I'm wondering if you can share some more specific pointers? > > Grepping for "self-refresh" yields references to the eDP self refresh, > but that seems not relevant as that one depends on hardware to decide > when to enter the self-refresh mode. > > Perhaps I was not looking at the right place (I searched git and > patchwork), but I was not able to find work that would seem relevant. self-refresh is a generic term not tied to eDP. It just means that the panel can refresh itself, and doesn't need to be fed the pixel data all the time (as long as nothing changes), allowing the display hw (and in most cases also the entire SoC) to be shut down. How exactly it's implemented is up to the panel/display hw. For eDP there's no requirement that the hw detects changes, that's just how i915 implements it. Afaik other drivers implemented self-refresh entirely using software logic. Self-refresh also exists for dsi, and the olpc hw you have here (and the fbdev driver thing I've recently found) seems to fit that paradigm too. -Daniel
On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote: > 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). Hi, Would it be possible for you to re-post patches 1 through 6 to include the DT maintainers so that they can review those patches (as is required for their acceptance) - once they have done that, I can pick them up and resubmit some of the patches I have queued. Thanks.
On Thu, 2019-01-17 at 10:55 +0000, Russell King - ARM Linux admin wrote: > On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote: > > 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). > > Hi, > > Would it be possible for you to re-post patches 1 through 6 to include > the DT maintainers so that they can review those patches (as is > required for their acceptance) - once they have done that, I can pick > them up and resubmit some of the patches I have queued. Yes. Before I do so, I'd like to know your opinion about the compatible strings of the framebuffer and display-subsystem nodes. In the "[RFC 05/16] dt-bindings: display: armada: Add mmp2 compatible strings" message [1] I wrote: > Note that perhaps "marvell,armada-display-subsystem" and > "marvell,armada-framebuffer" would be a good idea in addition of > dove/mmp2 specific strings since (at least for now) the driver code > is the same. [1] https://lists.freedesktop.org/archives/dri-devel/2018-December/201016.html I essentially documented what you implemented, but I'd prefer if you used the "marvell,armada-display-subsystem" and "marvell,armada-framebuffer" compatible strings in the driver code instead. What do you think? > > Thanks. Thank you Lubo
On Thu, Jan 17, 2019 at 04:03:45PM +0100, Lubomir Rintel wrote: > On Thu, 2019-01-17 at 10:55 +0000, Russell King - ARM Linux admin > wrote: > > On Tue, Dec 18, 2018 at 04:37:26PM +0100, Lubomir Rintel wrote: > > > 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). > > > > Hi, > > > > Would it be possible for you to re-post patches 1 through 6 to include > > the DT maintainers so that they can review those patches (as is > > required for their acceptance) - once they have done that, I can pick > > them up and resubmit some of the patches I have queued. > > Yes. Before I do so, I'd like to know your opinion about the compatible > strings of the framebuffer and display-subsystem nodes. > > In the "[RFC 05/16] dt-bindings: display: armada: Add mmp2 compatible > strings" message [1] I wrote: > > Note that perhaps "marvell,armada-display-subsystem" and > > "marvell,armada-framebuffer" would be a good idea in addition of > > dove/mmp2 specific strings since (at least for now) the driver code > > is the same. > > [1] https://lists.freedesktop.org/archives/dri-devel/2018-December/201016.html > > I essentially documented what you implemented, but I'd prefer if you > used the "marvell,armada-display-subsystem" and > "marvell,armada-framebuffer" compatible strings in the driver code > instead. Things in DT tend to be named by the first user who comes along - so we often get DT compatibles that refer to the first device. However, we do currently have the opportunity to change it, so please do as you suggest.
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? 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? 3.) My patch set currently contains the driver for the Himax that is modelled after tda998x. That one implements an encoder. Similar drivers seem to add a bridge too, but it's not entirely clear to me what a bridge is good for? 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? Thanks, Lubo