Re: [RFC 0/3] solving omapdrm/omapdss layering issues
Hi Rob, Tomi, On Thu, Aug 2, 2012 at 7:46 PM, Rob Clark rob.cl...@linaro.org wrote: On Thu, Aug 2, 2012 at 8:15 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) Hm, I'm not sure I understand, omapdss concepts map directly to the hardware. I think it is mainly exposing the encoder and panel as two separate entities.. which seems to be what Archit is working on I still don't follow =) They are separate entities. Omapdss models the HW quite directly, I think. It doesn't expose everything, though, as the output drivers (dsi.c, dpi.c etc) are used via the panel drivers. right.. so we just need to expose the output drivers as separate entities, and let omapdrm propagate information such as timings between them in case of something like DVI bridge from DPI, this seems pretty straightforward.. only the connector needs to know about DDC stuff, which i2c to use and that sort of thing. So at kms level we would have (for example) an omap_dpi_encoder which would be the same for DPI panel (connector) or DPI-DVI bridge. For HDMI I'm still looking through the code to see how this would work. Honestly I've looked less at this part of code and encoder related registers in the TRM, compared to the ovl/mgr parts, but at least from the 'DSS overview' picture in the TRM it seems to make sense ;-) KMS even exposes the idea that certain crtcs can connect to only certain encoders. Or that you could you could have certain connectors switched between encoders. For example if you had a hw w/ DPI out, and some mux to switch that back and forth between a DPI lcd panel and a DPI-DVI bridge. (Ok, I'm not aware of any board that actually does this, but it is in theory possible.) So we could expose possible video chain topologies to userspace in this way. OMAP3 SDP board has such a setup, with manual switch to select between LCD and DVI. ahh, good to know.. so I'm not crazy for wanting to expose this possibility to userspace The other thing is that we don't need to propagate timings from the panel up to the mgr at the dss level.. kms is already handling this for us. In my latest version, which I haven't pushed, I removed the 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. You're thinking only about simple DPI cases. Consider this example, with a DSI-to-DP bridge chip. What we have is the following flow of data: DISPC - DSI - DSI-2-DP - DP monitor The timings you are thinking about are in the DISPC, but here they are only one part of the link. And here the DISPC timings are not actually the timings what the user is interested about. The user wants his timings to be between DSI-2-DP chip and the DP monitor. Timings programmed to DISPC are not the same. The timings for DISPC come from the DSI driver, and they may be very different than the user's timings. With DSI video mode, the DISPC timings would have some resemblance to the user's timings, mainly the time to send one line would be the same. With DSI cmd mode, the DISPC timings would be something totally different, most likely with 0 blank times and as fast pixel clock as possible. hmm, well kms already has a concept of adjusted_timings, which could perhaps be used here to propagate the timings between crtc-encoder.. although the order is probably backwards from what we want (it comes from the crtc to the encoder.. and if I understand properly we want it the other way and actually possibly from the connector). But that isn't to say that internally in omapdrm the crtc couldn't get the adjusted timings from the connector. So I still think the parameter flow doesn't need to be 'under the hood' in omapdss. And fwiw, the adjusted_timings stuff is handled by drm_crtc_helper fxns, so if the way the core kms handles it isn't what we want, we can just plug in our own fxn instead of using drm_crtc_helper_set_mode(), so that isn't really a big problem. What omapdss does currently is that you set the user's timings to the right side of the chain, which propagate back to DSS. This allows the DSI-2-DP bridge use DSI timings that work optimally for the bridge, and DSI driver will use DISPC timings that work optimally for it. And it's not only about timings above, but also other settings related to the busses between the components. Clock dividers, polarities, stuff like that. I expect we could handle other settings in the same way as the timings. I think the problem was there were some
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Fri, Aug 3, 2012 at 1:01 AM, Semwal, Sumit sumit.sem...@ti.com wrote: Hi Rob, Tomi, On Thu, Aug 2, 2012 at 7:46 PM, Rob Clark rob.cl...@linaro.org wrote: On Thu, Aug 2, 2012 at 8:15 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) Hm, I'm not sure I understand, omapdss concepts map directly to the hardware. I think it is mainly exposing the encoder and panel as two separate entities.. which seems to be what Archit is working on I still don't follow =) They are separate entities. Omapdss models the HW quite directly, I think. It doesn't expose everything, though, as the output drivers (dsi.c, dpi.c etc) are used via the panel drivers. right.. so we just need to expose the output drivers as separate entities, and let omapdrm propagate information such as timings between them in case of something like DVI bridge from DPI, this seems pretty straightforward.. only the connector needs to know about DDC stuff, which i2c to use and that sort of thing. So at kms level we would have (for example) an omap_dpi_encoder which would be the same for DPI panel (connector) or DPI-DVI bridge. For HDMI I'm still looking through the code to see how this would work. Honestly I've looked less at this part of code and encoder related registers in the TRM, compared to the ovl/mgr parts, but at least from the 'DSS overview' picture in the TRM it seems to make sense ;-) KMS even exposes the idea that certain crtcs can connect to only certain encoders. Or that you could you could have certain connectors switched between encoders. For example if you had a hw w/ DPI out, and some mux to switch that back and forth between a DPI lcd panel and a DPI-DVI bridge. (Ok, I'm not aware of any board that actually does this, but it is in theory possible.) So we could expose possible video chain topologies to userspace in this way. OMAP3 SDP board has such a setup, with manual switch to select between LCD and DVI. ahh, good to know.. so I'm not crazy for wanting to expose this possibility to userspace The other thing is that we don't need to propagate timings from the panel up to the mgr at the dss level.. kms is already handling this for us. In my latest version, which I haven't pushed, I removed the 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. You're thinking only about simple DPI cases. Consider this example, with a DSI-to-DP bridge chip. What we have is the following flow of data: DISPC - DSI - DSI-2-DP - DP monitor The timings you are thinking about are in the DISPC, but here they are only one part of the link. And here the DISPC timings are not actually the timings what the user is interested about. The user wants his timings to be between DSI-2-DP chip and the DP monitor. Timings programmed to DISPC are not the same. The timings for DISPC come from the DSI driver, and they may be very different than the user's timings. With DSI video mode, the DISPC timings would have some resemblance to the user's timings, mainly the time to send one line would be the same. With DSI cmd mode, the DISPC timings would be something totally different, most likely with 0 blank times and as fast pixel clock as possible. hmm, well kms already has a concept of adjusted_timings, which could perhaps be used here to propagate the timings between crtc-encoder.. although the order is probably backwards from what we want (it comes from the crtc to the encoder.. and if I understand properly we want it the other way and actually possibly from the connector). But that isn't to say that internally in omapdrm the crtc couldn't get the adjusted timings from the connector. So I still think the parameter flow doesn't need to be 'under the hood' in omapdss. And fwiw, the adjusted_timings stuff is handled by drm_crtc_helper fxns, so if the way the core kms handles it isn't what we want, we can just plug in our own fxn instead of using drm_crtc_helper_set_mode(), so that isn't really a big problem. What omapdss does currently is that you set the user's timings to the right side of the chain, which propagate back to DSS. This allows the DSI-2-DP bridge use DSI timings that work optimally for the bridge, and DSI driver will use DISPC timings that work optimally for it. And it's not only about timings above, but also other settings related to the busses between the components. Clock dividers, polarities, stuff like that. I expect we could handle other settings
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) Hm, I'm not sure I understand, omapdss concepts map directly to the hardware. The one area that kms mismatches a bit is decoupling of ovl from mgr that we have in our hw.. I've partially solved that a while back w/ What do you mean with that? Ovls and mgrs are one entity in KMS? Didn't the drm_plane stuff separate these? For enabling/disabling an output (manager+encoder), this is relatively infrequent, so it can afford to block to avoid races. (Like userspace enabling and then rapidly disabling an output part way through the enable.) But enabling/disabling an overlay, or adjusting position or scanout address must not block. And ideally, if possible, switching an overlay between two managers should not block. Adjusting the position or address of the buffer are simple, they can be easily done without any blocking. But ovl enable/disable and switching an ovl to another mgr do (possibly) take multiple vsyncs (and in the switch case, vsyncs of two separate outputs). So if those do not block, we'll need to handle them as a state machine and try to avoid races etc. It'll be interesting. However, we can sometimes do those operations immediately. So I think we should have these conditional fast-paths in the code, and do them in non-blocking manner when possible. I'll look again, but as far as I remember that at least wasn't addressing the performance issues from making overlay enable/disable Right, it wasn't addressing those issues. But your branch doesn't really address those issues either, as it doesn't handle the problems related to ovl enable/disable. synchronous. And fixing that would, I expect, trigger the same problems that I already spent a few days debugging before switching over to handle apply in omapdrm. I was under the impression that the apply mechanism, the caching and setting of the configs, was the major issue you had. But you're hinting that the actual problem is related to ovl enable/disable? I haven't tried fixing the ovl enable/disable, as I didn't know it's an issue for omapdrm. Or are they both as big issues? Tomi signature.asc Description: This is a digitally signed message part
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) Hm, I'm not sure I understand, omapdss concepts map directly to the hardware. I think it is mainly exposing the encoder and panel as two separate entities.. which seems to be what Archit is working on I still don't follow =) They are separate entities. Omapdss models the HW quite directly, I think. It doesn't expose everything, though, as the output drivers (dsi.c, dpi.c etc) are used via the panel drivers. in case of something like DVI bridge from DPI, this seems pretty straightforward.. only the connector needs to know about DDC stuff, which i2c to use and that sort of thing. So at kms level we would have (for example) an omap_dpi_encoder which would be the same for DPI panel (connector) or DPI-DVI bridge. For HDMI I'm still looking through the code to see how this would work. Honestly I've looked less at this part of code and encoder related registers in the TRM, compared to the ovl/mgr parts, but at least from the 'DSS overview' picture in the TRM it seems to make sense ;-) KMS even exposes the idea that certain crtcs can connect to only certain encoders. Or that you could you could have certain connectors switched between encoders. For example if you had a hw w/ DPI out, and some mux to switch that back and forth between a DPI lcd panel and a DPI-DVI bridge. (Ok, I'm not aware of any board that actually does this, but it is in theory possible.) So we could expose possible video chain topologies to userspace in this way. OMAP3 SDP board has such a setup, with manual switch to select between LCD and DVI. The other thing is that we don't need to propagate timings from the panel up to the mgr at the dss level.. kms is already handling this for us. In my latest version, which I haven't pushed, I removed the 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. You're thinking only about simple DPI cases. Consider this example, with a DSI-to-DP bridge chip. What we have is the following flow of data: DISPC - DSI - DSI-2-DP - DP monitor The timings you are thinking about are in the DISPC, but here they are only one part of the link. And here the DISPC timings are not actually the timings what the user is interested about. The user wants his timings to be between DSI-2-DP chip and the DP monitor. Timings programmed to DISPC are not the same. The timings for DISPC come from the DSI driver, and they may be very different than the user's timings. With DSI video mode, the DISPC timings would have some resemblance to the user's timings, mainly the time to send one line would be the same. With DSI cmd mode, the DISPC timings would be something totally different, most likely with 0 blank times and as fast pixel clock as possible. What omapdss does currently is that you set the user's timings to the right side of the chain, which propagate back to DSS. This allows the DSI-2-DP bridge use DSI timings that work optimally for the bridge, and DSI driver will use DISPC timings that work optimally for it. And it's not only about timings above, but also other settings related to the busses between the components. Clock dividers, polarities, stuff like that. I think the problem was there were some cases, like ovl updates before setting the mgr, where the user_info_dirty flag would be cleared but the registers not updated. This is what I meant by sequence of Hmm, I'd appreciate more info about this if you can give. Sounds like a bug, that shouldn't happen. So you mean that you have an ovl, with no manager set. And you change the settings of the ovl before assigning it to a mgr? That's something I haven't really tested, so it could bug, true. operations at KMS level confusing omapdss. This should be fixable with some debugging. Although getting rid of the state tracking at omapdss level altogether was a much simpler solution, and is the one I prefer :-) Yes, I don't prefer the state tracking either (we didn't have it in earlier versions of omapdss), but I still don't see an option to it if we want to support all the stuff we have. Tomi signature.asc Description: This is a digitally signed message part
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Thu, Aug 2, 2012 at 8:15 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Thu, 2012-08-02 at 07:45 -0500, Rob Clark wrote: On Thu, Aug 2, 2012 at 2:13 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-08-01 at 09:25 -0500, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) Hm, I'm not sure I understand, omapdss concepts map directly to the hardware. I think it is mainly exposing the encoder and panel as two separate entities.. which seems to be what Archit is working on I still don't follow =) They are separate entities. Omapdss models the HW quite directly, I think. It doesn't expose everything, though, as the output drivers (dsi.c, dpi.c etc) are used via the panel drivers. right.. so we just need to expose the output drivers as separate entities, and let omapdrm propagate information such as timings between them in case of something like DVI bridge from DPI, this seems pretty straightforward.. only the connector needs to know about DDC stuff, which i2c to use and that sort of thing. So at kms level we would have (for example) an omap_dpi_encoder which would be the same for DPI panel (connector) or DPI-DVI bridge. For HDMI I'm still looking through the code to see how this would work. Honestly I've looked less at this part of code and encoder related registers in the TRM, compared to the ovl/mgr parts, but at least from the 'DSS overview' picture in the TRM it seems to make sense ;-) KMS even exposes the idea that certain crtcs can connect to only certain encoders. Or that you could you could have certain connectors switched between encoders. For example if you had a hw w/ DPI out, and some mux to switch that back and forth between a DPI lcd panel and a DPI-DVI bridge. (Ok, I'm not aware of any board that actually does this, but it is in theory possible.) So we could expose possible video chain topologies to userspace in this way. OMAP3 SDP board has such a setup, with manual switch to select between LCD and DVI. ahh, good to know.. so I'm not crazy for wanting to expose this possibility to userspace The other thing is that we don't need to propagate timings from the panel up to the mgr at the dss level.. kms is already handling this for us. In my latest version, which I haven't pushed, I removed the 'struct omap_overlay_mgr' ptr from 'struct omap_dss_device'. You're thinking only about simple DPI cases. Consider this example, with a DSI-to-DP bridge chip. What we have is the following flow of data: DISPC - DSI - DSI-2-DP - DP monitor The timings you are thinking about are in the DISPC, but here they are only one part of the link. And here the DISPC timings are not actually the timings what the user is interested about. The user wants his timings to be between DSI-2-DP chip and the DP monitor. Timings programmed to DISPC are not the same. The timings for DISPC come from the DSI driver, and they may be very different than the user's timings. With DSI video mode, the DISPC timings would have some resemblance to the user's timings, mainly the time to send one line would be the same. With DSI cmd mode, the DISPC timings would be something totally different, most likely with 0 blank times and as fast pixel clock as possible. hmm, well kms already has a concept of adjusted_timings, which could perhaps be used here to propagate the timings between crtc-encoder.. although the order is probably backwards from what we want (it comes from the crtc to the encoder.. and if I understand properly we want it the other way and actually possibly from the connector). But that isn't to say that internally in omapdrm the crtc couldn't get the adjusted timings from the connector. So I still think the parameter flow doesn't need to be 'under the hood' in omapdss. And fwiw, the adjusted_timings stuff is handled by drm_crtc_helper fxns, so if the way the core kms handles it isn't what we want, we can just plug in our own fxn instead of using drm_crtc_helper_set_mode(), so that isn't really a big problem. What omapdss does currently is that you set the user's timings to the right side of the chain, which propagate back to DSS. This allows the DSI-2-DP bridge use DSI timings that work optimally for the bridge, and DSI driver will use DISPC timings that work optimally for it. And it's not only about timings above, but also other settings related to the busses between the components. Clock dividers, polarities, stuff like that. I expect we could handle other settings in the same way as the timings. I think the problem was there were some cases, like ovl updates before setting the mgr, where the user_info_dirty flag would be cleared but the
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Tue, 2012-07-31 at 09:45 -0500, Rob Clark wrote: On Tue, Jul 31, 2012 at 8:40 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: It's not really about being friendly. Omapdss tries to do as little as possible, while still supporting all its HW features. Shadow registers are a bit tricky creating this mess. What I mean by 'friendly' is it tries to abstract this for simple users, like an fbdev driver. But this really quickly breaks down w/ a No, that's not what omapdss tries to do. I'm not trying to hide the shadow registers and the GO bit behind the omapdss API, I'm just trying to make it work. The omapdss API was made with omapfb, so it's true that the API may not be good for omapdrm. But I'm happy to change the API. And btw, I think the current mapping of drm_encoder to mgr in omapdrm is not correct. I'm just in the process of shuffling things around. I think drm/kms actually maps quite nicely to the underlying hardware with the following arrangement: drm_plane - ovl drm_crtc - mgr drm_encoder - DSI/DPI/HDMI/VENC encoder drm_connector - pretty much what we call a panel driver today Hmm, what was the arrangement earlier? I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. It would be quite useful if you could look at the omap_drm_apply mechanism I had in omapdrm, because that seems like a quite straightforward way to deal w/ shadowed registers. I think it will Yes, it seems straightforward, but it's not =). I had a look at your omapdrm-on-dispc-2 branch. What you are doing there is quite similar to what omapdss was doing earlier. It's not going to work reliably with multiple outputs and fifomerge. Configuring things like overlay color mode are quite simple. They only affect that one overlay. Also things like manager default bg color are simple, they affect only that one manager. But enabling/disabling an overlay or a manager, changing the destination mgr of an overlay, fifomerge... Those are not simple. You can't do them directly, as you do in your branch. As an example, consider the case of enabling an overlay (vid1), and moving fifo buffers from currently enabled overlay (gfx) to vid1: you'll first need to take the fifo buffers from gfx, set GO, and wait for the settings to take effect. Only then you can set the fifo buffers for vid1, enable it and set GO bit. I didn't write omapdss's apply.c for fun or to make omapfb simpler. I made it because the shadow register system is complex, and we need to handle the tricky cases somewhere. So, as I said before, I believe you'll just end up writing similar code to what is currently in apply.c. It won't be as simple as your current branch. Also, as I mentioned earlier, you'll also need to handle the output side of the shadow registers. These come from the output drivers (DPI, DSI, etc, and indirectly from panel drivers). They are not currently handled in the best manner in omapdss, but Archit is working on that and in his version apply.c will handle also those properly. About your code, I see you have these pre and post apply callbacks that handle the configuration. Wouldn't it be rather easy to have omapdss's apply.c call these? And then one thing I don't think you've considered is manual update displays. Of course, one option is to not support those with omapdrm, but that's quite a big decision. omapdss's apply.c handles those also. Also, can you check again my mail Re: OMAPDSS vsyncs/apply Sat, 12 May 2012 10:01:24, about the request_config() suggestion. I think that would be somewhat similar to your pre/post callbacks. I'll try to write some prototype for the request_config suggestion so that it's easier to understand. Tomi signature.asc Description: This is a digitally signed message part
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Tue, 2012-07-31 at 09:45 -0500, Rob Clark wrote: On Tue, Jul 31, 2012 at 8:40 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: It's not really about being friendly. Omapdss tries to do as little as possible, while still supporting all its HW features. Shadow registers are a bit tricky creating this mess. What I mean by 'friendly' is it tries to abstract this for simple users, like an fbdev driver. But this really quickly breaks down w/ a No, that's not what omapdss tries to do. I'm not trying to hide the shadow registers and the GO bit behind the omapdss API, I'm just trying to make it work. The omapdss API was made with omapfb, so it's true that the API may not be good for omapdrm. But I'm happy to change the API. And btw, I think the current mapping of drm_encoder to mgr in omapdrm is not correct. I'm just in the process of shuffling things around. I think drm/kms actually maps quite nicely to the underlying hardware with the following arrangement: drm_plane - ovl drm_crtc - mgr drm_encoder - DSI/DPI/HDMI/VENC encoder drm_connector - pretty much what we call a panel driver today Hmm, what was the arrangement earlier? it was previously: plane - ovl crtc - placeholder encoder - mgr connector - dssdev (encoder+panel) although crtc is really the point where you should enable/disable vblank irqs, so the new arrangement is somewhat cleaner (although on my branch the encoder/connector part are not finished yet) I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) The one area that kms mismatches a bit is decoupling of ovl from mgr that we have in our hw.. I've partially solved that a while back w/ the patch in drm to add private planes so the omap_crtc internally uses an omap_plane. It isn't exposed to userspace to be able to re-use the planes from unused crtcs, although I have some ideas about that (but not yet time to work on it). It would be quite useful if you could look at the omap_drm_apply mechanism I had in omapdrm, because that seems like a quite straightforward way to deal w/ shadowed registers. I think it will Yes, it seems straightforward, but it's not =). I had a look at your omapdrm-on-dispc-2 branch. What you are doing there is quite similar to what omapdss was doing earlier. It's not going to work reliably with multiple outputs and fifomerge. Configuring things like overlay color mode are quite simple. They only affect that one overlay. Also things like manager default bg color are simple, they affect only that one manager. But enabling/disabling an overlay or a manager, changing the destination mgr of an overlay, fifomerge... Those are not simple. You can't do them directly, as you do in your branch. As an example, consider the case of enabling an overlay (vid1), and moving fifo buffers from currently enabled overlay (gfx) to vid1: you'll first need to take the fifo buffers from gfx, set GO, and wait for the settings to take effect. Only then you can set the fifo buffers for vid1, enable it and set GO bit. hmm, it does sound like it needs a bit of a state machine to deal with multi-step updates.. although that makes races more of a problem, which was something I was trying hard to avoid. For enabling/disabling an output (manager+encoder), this is relatively infrequent, so it can afford to block to avoid races. (Like userspace enabling and then rapidly disabling an output part way through the enable.) But enabling/disabling an overlay, or adjusting position or scanout address must not block. And ideally, if possible, switching an overlay between two managers should not block. For fifomerge, if I understand correctly, it shouldn't really be needed for functionality, but mainly as a power optimization? If this is the case I wonder about an approach of disabling fifomerge when there are ongoing setting changes, and then setting it after things settle down? I'll have to think about it, but I was trying to avoid needing a multi-step state machine to avoid the associated race conditions, but if this is not possible then it is not possible. I didn't write omapdss's apply.c for fun or to make omapfb simpler. I made it because the shadow register system is complex, and we need to handle the tricky cases somewhere. So, as I said before, I believe you'll just end up writing similar code to what is currently in apply.c. It won't be as simple as your current branch. Also, as I mentioned earlier, you'll also need to handle the output side of the shadow registers. These come from the output drivers (DPI, DSI, etc, and indirectly from panel drivers). They are not currently handled in the best manner in omapdss, but Archit is working on that and in his
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
Hi, On Wednesday 01 August 2012 07:55 PM, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Tue, 2012-07-31 at 09:45 -0500, Rob Clark wrote: On Tue, Jul 31, 2012 at 8:40 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: It's not really about being friendly. Omapdss tries to do as little as possible, while still supporting all its HW features. Shadow registers are a bit tricky creating this mess. What I mean by 'friendly' is it tries to abstract this for simple users, like an fbdev driver. But this really quickly breaks down w/ a No, that's not what omapdss tries to do. I'm not trying to hide the shadow registers and the GO bit behind the omapdss API, I'm just trying to make it work. The omapdss API was made with omapfb, so it's true that the API may not be good for omapdrm. But I'm happy to change the API. And btw, I think the current mapping of drm_encoder to mgr in omapdrm is not correct. I'm just in the process of shuffling things around. I think drm/kms actually maps quite nicely to the underlying hardware with the following arrangement: drm_plane - ovl drm_crtc - mgr drm_encoder - DSI/DPI/HDMI/VENC encoder drm_connector - pretty much what we call a panel driver today Hmm, what was the arrangement earlier? it was previously: plane - ovl crtc - placeholder encoder - mgr connector - dssdev (encoder+panel) although crtc is really the point where you should enable/disable vblank irqs, so the new arrangement is somewhat cleaner (although on my branch the encoder/connector part are not finished yet) I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) The one area that kms mismatches a bit is decoupling of ovl from mgr that we have in our hw.. I've partially solved that a while back w/ the patch in drm to add private planes so the omap_crtc internally uses an omap_plane. It isn't exposed to userspace to be able to re-use the planes from unused crtcs, although I have some ideas about that (but not yet time to work on it). It would be quite useful if you could look at the omap_drm_apply mechanism I had in omapdrm, because that seems like a quite straightforward way to deal w/ shadowed registers. I think it will Yes, it seems straightforward, but it's not =). I had a look at your omapdrm-on-dispc-2 branch. What you are doing there is quite similar to what omapdss was doing earlier. It's not going to work reliably with multiple outputs and fifomerge. Configuring things like overlay color mode are quite simple. They only affect that one overlay. Also things like manager default bg color are simple, they affect only that one manager. But enabling/disabling an overlay or a manager, changing the destination mgr of an overlay, fifomerge... Those are not simple. You can't do them directly, as you do in your branch. As an example, consider the case of enabling an overlay (vid1), and moving fifo buffers from currently enabled overlay (gfx) to vid1: you'll first need to take the fifo buffers from gfx, set GO, and wait for the settings to take effect. Only then you can set the fifo buffers for vid1, enable it and set GO bit. hmm, it does sound like it needs a bit of a state machine to deal with multi-step updates.. although that makes races more of a problem, which was something I was trying hard to avoid. For enabling/disabling an output (manager+encoder), this is relatively infrequent, so it can afford to block to avoid races. (Like userspace enabling and then rapidly disabling an output part way through the enable.) But enabling/disabling an overlay, or adjusting position or scanout address must not block. And ideally, if possible, switching an overlay between two managers should not block. For fifomerge, if I understand correctly, it shouldn't really be needed for functionality, but mainly as a power optimization? If this is the case I wonder about an approach of disabling fifomerge when there are ongoing setting changes, and then setting it after things settle down? I'll have to think about it, but I was trying to avoid needing a multi-step state machine to avoid the associated race conditions, but if this is not possible then it is not possible. I didn't write omapdss's apply.c for fun or to make omapfb simpler. I made it because the shadow register system is complex, and we need to handle the tricky cases somewhere. So, as I said before, I believe you'll just end up writing similar code to what is currently in apply.c. It won't be as simple as your current branch. Also, as I mentioned earlier, you'll also need to handle the output side of the shadow registers. These come from the output drivers (DPI, DSI, etc, and indirectly from panel drivers). They are not currently handled in the best manner in omapdss, but Archit is
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, Aug 1, 2012 at 11:46 AM, Archit Taneja arc...@ti.com wrote: Hi, On Wednesday 01 August 2012 07:55 PM, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Tue, 2012-07-31 at 09:45 -0500, Rob Clark wrote: On Tue, Jul 31, 2012 at 8:40 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: It's not really about being friendly. Omapdss tries to do as little as possible, while still supporting all its HW features. Shadow registers are a bit tricky creating this mess. What I mean by 'friendly' is it tries to abstract this for simple users, like an fbdev driver. But this really quickly breaks down w/ a No, that's not what omapdss tries to do. I'm not trying to hide the shadow registers and the GO bit behind the omapdss API, I'm just trying to make it work. The omapdss API was made with omapfb, so it's true that the API may not be good for omapdrm. But I'm happy to change the API. And btw, I think the current mapping of drm_encoder to mgr in omapdrm is not correct. I'm just in the process of shuffling things around. I think drm/kms actually maps quite nicely to the underlying hardware with the following arrangement: drm_plane - ovl drm_crtc - mgr drm_encoder - DSI/DPI/HDMI/VENC encoder drm_connector - pretty much what we call a panel driver today Hmm, what was the arrangement earlier? it was previously: plane - ovl crtc - placeholder encoder - mgr connector - dssdev (encoder+panel) although crtc is really the point where you should enable/disable vblank irqs, so the new arrangement is somewhat cleaner (although on my branch the encoder/connector part are not finished yet) I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) The one area that kms mismatches a bit is decoupling of ovl from mgr that we have in our hw.. I've partially solved that a while back w/ the patch in drm to add private planes so the omap_crtc internally uses an omap_plane. It isn't exposed to userspace to be able to re-use the planes from unused crtcs, although I have some ideas about that (but not yet time to work on it). It would be quite useful if you could look at the omap_drm_apply mechanism I had in omapdrm, because that seems like a quite straightforward way to deal w/ shadowed registers. I think it will Yes, it seems straightforward, but it's not =). I had a look at your omapdrm-on-dispc-2 branch. What you are doing there is quite similar to what omapdss was doing earlier. It's not going to work reliably with multiple outputs and fifomerge. Configuring things like overlay color mode are quite simple. They only affect that one overlay. Also things like manager default bg color are simple, they affect only that one manager. But enabling/disabling an overlay or a manager, changing the destination mgr of an overlay, fifomerge... Those are not simple. You can't do them directly, as you do in your branch. As an example, consider the case of enabling an overlay (vid1), and moving fifo buffers from currently enabled overlay (gfx) to vid1: you'll first need to take the fifo buffers from gfx, set GO, and wait for the settings to take effect. Only then you can set the fifo buffers for vid1, enable it and set GO bit. hmm, it does sound like it needs a bit of a state machine to deal with multi-step updates.. although that makes races more of a problem, which was something I was trying hard to avoid. For enabling/disabling an output (manager+encoder), this is relatively infrequent, so it can afford to block to avoid races. (Like userspace enabling and then rapidly disabling an output part way through the enable.) But enabling/disabling an overlay, or adjusting position or scanout address must not block. And ideally, if possible, switching an overlay between two managers should not block. For fifomerge, if I understand correctly, it shouldn't really be needed for functionality, but mainly as a power optimization? If this is the case I wonder about an approach of disabling fifomerge when there are ongoing setting changes, and then setting it after things settle down? I'll have to think about it, but I was trying to avoid needing a multi-step state machine to avoid the associated race conditions, but if this is not possible then it is not possible. I didn't write omapdss's apply.c for fun or to make omapfb simpler. I made it because the shadow register system is complex, and we need to handle the tricky cases somewhere. So, as I said before, I believe you'll just end up writing similar code to what is currently in apply.c. It won't be as simple as your current branch. Also, as I mentioned earlier, you'll also need to handle the output side of the shadow registers. These come from
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, Aug 1, 2012 at 11:53 AM, Rob Clark rob.cl...@linaro.org wrote: On Wed, Aug 1, 2012 at 11:46 AM, Archit Taneja arc...@ti.com wrote: Hi, On Wednesday 01 August 2012 07:55 PM, Rob Clark wrote: On Wed, Aug 1, 2012 at 4:21 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Tue, 2012-07-31 at 09:45 -0500, Rob Clark wrote: On Tue, Jul 31, 2012 at 8:40 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: It's not really about being friendly. Omapdss tries to do as little as possible, while still supporting all its HW features. Shadow registers are a bit tricky creating this mess. What I mean by 'friendly' is it tries to abstract this for simple users, like an fbdev driver. But this really quickly breaks down w/ a No, that's not what omapdss tries to do. I'm not trying to hide the shadow registers and the GO bit behind the omapdss API, I'm just trying to make it work. The omapdss API was made with omapfb, so it's true that the API may not be good for omapdrm. But I'm happy to change the API. And btw, I think the current mapping of drm_encoder to mgr in omapdrm is not correct. I'm just in the process of shuffling things around. I think drm/kms actually maps quite nicely to the underlying hardware with the following arrangement: drm_plane - ovl drm_crtc - mgr drm_encoder - DSI/DPI/HDMI/VENC encoder drm_connector - pretty much what we call a panel driver today Hmm, what was the arrangement earlier? it was previously: plane - ovl crtc - placeholder encoder - mgr connector - dssdev (encoder+panel) although crtc is really the point where you should enable/disable vblank irqs, so the new arrangement is somewhat cleaner (although on my branch the encoder/connector part are not finished yet) I guess the fact is that DRM concepts do not really match the OMAP DSS hardware, and we'll have to use whatever gives us least problems. Actually, I think it does map fairly well to the hardware.. at least more so than to omapdss ;-) The one area that kms mismatches a bit is decoupling of ovl from mgr that we have in our hw.. I've partially solved that a while back w/ the patch in drm to add private planes so the omap_crtc internally uses an omap_plane. It isn't exposed to userspace to be able to re-use the planes from unused crtcs, although I have some ideas about that (but not yet time to work on it). It would be quite useful if you could look at the omap_drm_apply mechanism I had in omapdrm, because that seems like a quite straightforward way to deal w/ shadowed registers. I think it will Yes, it seems straightforward, but it's not =). I had a look at your omapdrm-on-dispc-2 branch. What you are doing there is quite similar to what omapdss was doing earlier. It's not going to work reliably with multiple outputs and fifomerge. Configuring things like overlay color mode are quite simple. They only affect that one overlay. Also things like manager default bg color are simple, they affect only that one manager. But enabling/disabling an overlay or a manager, changing the destination mgr of an overlay, fifomerge... Those are not simple. You can't do them directly, as you do in your branch. As an example, consider the case of enabling an overlay (vid1), and moving fifo buffers from currently enabled overlay (gfx) to vid1: you'll first need to take the fifo buffers from gfx, set GO, and wait for the settings to take effect. Only then you can set the fifo buffers for vid1, enable it and set GO bit. hmm, it does sound like it needs a bit of a state machine to deal with multi-step updates.. although that makes races more of a problem, which was something I was trying hard to avoid. For enabling/disabling an output (manager+encoder), this is relatively infrequent, so it can afford to block to avoid races. (Like userspace enabling and then rapidly disabling an output part way through the enable.) But enabling/disabling an overlay, or adjusting position or scanout address must not block. And ideally, if possible, switching an overlay between two managers should not block. For fifomerge, if I understand correctly, it shouldn't really be needed for functionality, but mainly as a power optimization? If this is the case I wonder about an approach of disabling fifomerge when there are ongoing setting changes, and then setting it after things settle down? I'll have to think about it, but I was trying to avoid needing a multi-step state machine to avoid the associated race conditions, but if this is not possible then it is not possible. I didn't write omapdss's apply.c for fun or to make omapfb simpler. I made it because the shadow register system is complex, and we need to handle the tricky cases somewhere. So, as I said before, I believe you'll just end up writing similar code to what is currently in apply.c. It won't be as simple as your current branch. Also, as I mentioned earlier, you'll also
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, 2012-08-01 at 11:53 -0500, Rob Clark wrote: Ok.. this would help. I'll take a look. I do request that interfaces/panels don't set any mgr/timing related registers. I had to comment all this stuff out in my prototype. Really we want to set the timings separately on the crtc (mgr) / encoder (interface) / connector (panel.. not sure if it is needed, though?). KMS will take care of propagating the timings through the pipeline. If we only had auto-update displays, and only the video timings were shadow register, it'd work. Unfortunately we have other registers as shadow registers also, like DISPC_CONTROL1, DISPC_CONFIG1 and DISPC_DIVISOR1. But we should think if this could be somehow be changed, so that all the shadow register info would come from one place. I do find it a bit unlikely with a quick thought, though. Well, hmm. Perhaps... Omapdrm (or omapfb etc) doesn't really need to know about the values of those registers, it just needs to control the GO bit. So perhaps if we had some method to inform omapdrm that these things have changed, and omapdrm would then set the GO bit as soon as possible. But there are some tricky stuff, like the divisors... Well, we need to think about this. Tomi signature.asc Description: This is a digitally signed message part
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Wed, Aug 1, 2012 at 12:38 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: On Wed, 2012-08-01 at 11:53 -0500, Rob Clark wrote: Ok.. this would help. I'll take a look. I do request that interfaces/panels don't set any mgr/timing related registers. I had to comment all this stuff out in my prototype. Really we want to set the timings separately on the crtc (mgr) / encoder (interface) / connector (panel.. not sure if it is needed, though?). KMS will take care of propagating the timings through the pipeline. If we only had auto-update displays, and only the video timings were shadow register, it'd work. Unfortunately we have other registers as shadow registers also, like DISPC_CONTROL1, DISPC_CONFIG1 and DISPC_DIVISOR1. well, I was kinda thinking we just do all the register access from the corresponding crtc (mgr)'s GO/apply sequencing.. so if, for example, you change resolution, then the plane, crtc, encoder, panel all queue up via omap_crtc_apply() on their associated crtc, and then at the right time from pre_apply() fxns call the appropriate omapdss/dispc function(s) for register updates. I think that would work well for everything but mgr enable/disable (which is infrequent, so ok to block for a few vblanks), and fifomerge, which I'm a bit on the fence about. But we should think if this could be somehow be changed, so that all the shadow register info would come from one place. I do find it a bit unlikely with a quick thought, though. Well, hmm. Perhaps... Omapdrm (or omapfb etc) doesn't really need to know about the values of those registers, it just needs to control the GO bit. So perhaps if we had some method to inform omapdrm that these things have changed, and omapdrm would then set the GO bit as soon as possible. Well, what I'm doing now is basically, if I update anything in any of the omap_*_info structs, I schedule an apply, and from pre_apply callback push the changes down to dispc.. I was thinking to follow the same for encoder and probably connector. (Not sure if doing things like setting timings at hdmi need to be GO bit sync'd? Maybe this could be bypassed for the connector, but if not it is easy enough just to use the same mechanism that the plane/crtc/encoder are already using.) But there are some tricky stuff, like the divisors... Well, we need to think about this. I think, if I understand properly, the most tricky thing is the shared clocks.. although I'm not really sure if we can actually change things like the core clock when you plug in a 2nd display w/out loosing sync on the first? That's seems like a tricky thing either way. Anything else, that is only effecting a single crtc-encoder-connector chain can have register programming sync'd to that mgr's GO bit. BR, -R Tomi -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
Hi, On Fri, 2012-07-27 at 20:07 -0500, Rob Clark wrote: From: Rob Clark r...@ti.com I've been working for the better part of the week on solving some of the omapdss vs kms mismatches, which is one of the bigger remaining issues in the TODO before moving omapdrm out of staging. The biggest place that this shows up is in GO bit handling. Basically, some of the scanout related registers in DSS hw block are only shadow registers, and if the GO bit is set during the vblank the hw copies into the actual registers (not accessible to CPU) and clears the GO bit. When the GO bit is set, there is no safe way to update these registers without undefined results. So omapdss tries to be friendly and abstract this, by buffering up the updated state, and applying it It's not really about being friendly. Omapdss tries to do as little as possible, while still supporting all its HW features. Shadow registers are a bit tricky creating this mess. on the next vblank once the GO bit is cleared. But this causes all sorts of mayhem at the omapdrm layer, which would like to unpin the previous scanout buffer(s) on the next vblank (or endwin) irq. Due to the buffering in omapdss, we have no way to know on a vblank if we have switched to the scanout buffer or not. Basically it works ok as long as userspace is only ever updating on layer (either crtc or drm plane) at a time. But throw together hw mouse cursor (drm plane) plus a window manager like compiz which does page flips, or wayland (weston drm compositor) with hw composition (drm plane), and things start to fail in a big way. I've tried a few approaches to preserve the omapdss more or less as it is, by adding callbacks for when GO bit is cleared, etc. But the sequencing of setting up connector/encoder/crtc is not really what omapdss expects, and would generally end up confusing the apply layer in omapdss (it would end up not programming various registers because various dirty flags would get cleared, for example mgr updated before overlay connected, etc). Can you give more info what the problem is? It shouldn't end up not programming registers, except if there's a bug there. Didn't the apply-id stuff I proposed some months ago have enough stuff to make this work? The thing about shadow registers is that we need to manage them in one central place. And the same shadow registers are used for both the composition stuff (overlays etc) and output stuff (video timings configuration). If omapdrm handles the composition shadow registers, it also needs to handle all the other shadow registers. Finally, in frustration, this afternoon I hit upon an idea. Why not just use the dispc code in omapdss, which is basically a stateless layer of helper functions, and bypass the stateful layer of omapdss. If you do this, you'll need to implement all the stuff of the stateful layer in omapdrm. You can't just call the dispc funcs and expect things to work reliably. Things like enabling/disabling overlays with fifomerge requires possibly multiple vsyncs. And output related shadow registers may be changed separately from the composition side. The apply.c is not there to make the life of the user of omapdss's easy, it's there to make the DSS hardware work properly =). Since KMS helper functions already give us the correct sequence for setting up the hardware, this turned out to be rather easy. And fit it quite nicely with my mechanism to queue up updates when the GO bit is not clear. And, unlike my previous attempts, it actually worked.. not only that, but it worked on the first boot! Well, not having read the omapdrm code, I'm not in the best position to comment, but I fear that while it seems to work, you have lots of corner cases where you'll get glitches. We had a lot simpler configuration model in the omapdss for long time, until the small corner cases started to pile up and new features started to cause problems, and I wrote the current apply mechanism. So I am pretty happy about how this is shaping up. Not only is it simpler that my previous attepmts, and solves a few tricky buffer unpin related issues. But it also makes it very easy to wire in the missing userspace vblank event handling without resorting to duct- tape. Why is giving an event from omapdss at vsync duct-tapy? Obviously there is stuff still missing, and some hacks. This is really just a proof of concept at this stage. But I wanted to send an RFC so we could start discussing how to move forward. Ie. could we reasonably add support to build dispc as a library of stateless helper functions, sharing it and the panel drivers between omapdrm and the legacy omapdss based drivers. Or is there no clean way to do that, in which case we should just copy the code we need into omapdrm, and leave the deprecated omapdss as it is for legacy drivers. I agree that we need to get omapdrm work well, as it's (or will be) the most important display framework. And if
Re: [RFC 0/3] solving omapdrm/omapdss layering issues
On Tue, Jul 31, 2012 at 8:40 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Hi, On Fri, 2012-07-27 at 20:07 -0500, Rob Clark wrote: From: Rob Clark r...@ti.com I've been working for the better part of the week on solving some of the omapdss vs kms mismatches, which is one of the bigger remaining issues in the TODO before moving omapdrm out of staging. The biggest place that this shows up is in GO bit handling. Basically, some of the scanout related registers in DSS hw block are only shadow registers, and if the GO bit is set during the vblank the hw copies into the actual registers (not accessible to CPU) and clears the GO bit. When the GO bit is set, there is no safe way to update these registers without undefined results. So omapdss tries to be friendly and abstract this, by buffering up the updated state, and applying it It's not really about being friendly. Omapdss tries to do as little as possible, while still supporting all its HW features. Shadow registers are a bit tricky creating this mess. What I mean by 'friendly' is it tries to abstract this for simple users, like an fbdev driver. But this really quickly breaks down w/ a more sophisticated user. Which is why I've been more in favor of making omapdss less of a layer. The idea of using it as some helper functions which handle a bit the variation of different generations of hw while not abstracting the fundamental operating concepts of DSS IP block (ie. GO bit stuff) seems perfect to me. So dispc plus dss_feature stuff seems like just what I'm looking for. on the next vblank once the GO bit is cleared. But this causes all sorts of mayhem at the omapdrm layer, which would like to unpin the previous scanout buffer(s) on the next vblank (or endwin) irq. Due to the buffering in omapdss, we have no way to know on a vblank if we have switched to the scanout buffer or not. Basically it works ok as long as userspace is only ever updating on layer (either crtc or drm plane) at a time. But throw together hw mouse cursor (drm plane) plus a window manager like compiz which does page flips, or wayland (weston drm compositor) with hw composition (drm plane), and things start to fail in a big way. I've tried a few approaches to preserve the omapdss more or less as it is, by adding callbacks for when GO bit is cleared, etc. But the sequencing of setting up connector/encoder/crtc is not really what omapdss expects, and would generally end up confusing the apply layer in omapdss (it would end up not programming various registers because various dirty flags would get cleared, for example mgr updated before overlay connected, etc). Can you give more info what the problem is? It shouldn't end up not programming registers, except if there's a bug there. Yeah, it is probably just a bug.. and a bug could be fixed. But with all that extra code it is certainly a lot harder to debug. So 'could' and 'should' are maybe two different things. Didn't the apply-id stuff I proposed some months ago have enough stuff to make this work? I guess the approach I was trying was similar to that proposal.. it probably could be made to work. But I am really not a big fan of unnecessary complexity. And unnecessary layering adds complexity. This is why in general most of the drm folks have preferred an approach of helper fxns rather than layers. And I tend to agree with them. The thing about shadow registers is that we need to manage them in one central place. And the same shadow registers are used for both the composition stuff (overlays etc) and output stuff (video timings configuration). If omapdrm handles the composition shadow registers, it also needs to handle all the other shadow registers. Yup.. I'm handling it in a central place :-) basically all the register programming is coming through the 'struct omap_drm_apply' mechanism, so it is all aligned to vblank/framedone and GO bit status. So probably that isn't strictly needed, because I'm treating all registers as shadow'd registers, but that seemed like a clean approach. Finally, in frustration, this afternoon I hit upon an idea. Why not just use the dispc code in omapdss, which is basically a stateless layer of helper functions, and bypass the stateful layer of omapdss. If you do this, you'll need to implement all the stuff of the stateful layer in omapdrm. You can't just call the dispc funcs and expect things to work reliably. Things like enabling/disabling overlays with fifomerge requires possibly multiple vsyncs. And output related shadow registers may be changed separately from the composition side. All the dispc fxn calls (except whatever is done directly from the 'omap_dss_device', which I haven't converted over yet) are done synchronized to the ovl mgrs GO bit. I haven't hooked up fifomerge yet, although looking at the apply code in omapdss, that looks like it should be pretty straightforward to hook into the same omap_drm_apply
