Re: [PATCH] OMAP: DSS2: DSI: Introduce sync_vc functions
On Thu, 2011-03-24 at 03:36 -0500, Taneja, Archit wrote: > Hi, > > On Thursday 24 March 2011 01:18 PM, Valkeinen, Tomi wrote: > > So how to solve that... > > > > Two ways come to my mind: > > - Track sent BTAs in dsi.c. Every time we send a packet, reset the > > counter. Every time we send a BTA, increase the counter. Thus at frame > > update we would know if we need to send an extra BTA. > > > > - Always reset the BTA "status" from the panel at the beginning of frame > > transfer. This could be done by sending a null packet. > > > > The second one is probably simpler and more failsafe as there's no state > > stored. The first one (as well as the current system) would go wrong if > > something strange happens, like the panel resets. However, the second > > one introduces some overhead, as we need to send a null packet and two > > BTAs (versus one BTA) for every frame. It's probably negligible, though. > > Okay. I agree the second one is a better option. I have a couple of > queries though: > -The second BTA should be sent only after we get the Ack for the first > one, i.e, we need to use bta_sync() for the first BTA, right? True. > -We shouldn't send null packets and the 2 BTAs at all if we aren't using > Automatic TE mode, is this correct? If by automatic TE mode you mean the DSI TEE trigger, then yes. There's currently check for the dsi.te_enabled in dsi_update_screen_dispc() for that. > -Whose job should it be to send the null packet and the 2 BTAs, the dsi > driver or the panel driver? I'd say the dsi driver. It currently sends the one BTA in dsi_update_screen_dispc(). Adding one null packet and a BTA there should be quite simple. 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: [PATCH] OMAP: DSS2: DSI: Introduce sync_vc functions
Hi, On Thursday 24 March 2011 01:18 PM, Valkeinen, Tomi wrote: On Wed, 2011-03-23 at 04:59 -0500, Taneja, Archit wrote: From: Archit Taneja The DSI protocol engine has no interrupt for signalling the end of a Frame transfer. The present approach is to send a BTA after DISPC generates a FRAMEDONE interrupt, and unlock the dsi bus only when the BTA Ack is received. The assumption made with this approach was that OMAP will send a BTA only after the long packet corresponding to the last line is sent. However, it is possible that on the DISPC FRAMEDONE interrupt there are 2 (or more) lines of pixel data in the DSI line buffer. Hence, the BTA Ack could be received for the long packet corresponding to the second last line (or the third last and so on..). Therefore, the current method doesn't ensure that the complete frame data is sent before we start a new transfer. A similar explanation holds valid if we send a BTA in between multiple short/long command packets from the slave port. Introduce dsi_sync_vc functions, based on Tomi Valkeinen's idea, which ensure that the DSI Virtual Channel in use(update_channel) completes its previous work before proceeding to the next Frame/Command. For a frame update, the DSI driver now sends a callback to the Panel Driver on the FRAMEDONE interrupt itself. The callback in the panel driver then unlocks the bus. dsi_sync_vc() functions are placed in dsi_vc_config_l4() and dsi_vc_config_vp() to ensure that the previous task of the Virtual Channel is completed. Signed-off-by: Archit Taneja --- Note: Applies over the master branch of the tree: http://gitorious.org/linux-omap-dss2/linux/commits/master drivers/video/omap2/dss/dsi.c | 180 ++-- 1 files changed, 117 insertions(+), 63 deletions(-) Looks good, but this patch introduces one problem: handling the double BTA for DSI TE. Currently we aim DSI to be always in a state where one BTA has been sent. So when the next frame transfer starts, we can just send one BTA and we know we'll get two consecutive BTAs. This patch removes the BTA which is sent after the frame transfer, thus breaking the above system. This doesn't cause problems currently because panel-taal.c always sends columns and rows addresses before starting the frame update, and a BTA is sent after those configurations. But a simple optimization would be to only send column/row config if they have changed. And in that case we would not get a double BTA, and thus we'd never get the DSI TE. So how to solve that... Two ways come to my mind: - Track sent BTAs in dsi.c. Every time we send a packet, reset the counter. Every time we send a BTA, increase the counter. Thus at frame update we would know if we need to send an extra BTA. - Always reset the BTA "status" from the panel at the beginning of frame transfer. This could be done by sending a null packet. The second one is probably simpler and more failsafe as there's no state stored. The first one (as well as the current system) would go wrong if something strange happens, like the panel resets. However, the second one introduces some overhead, as we need to send a null packet and two BTAs (versus one BTA) for every frame. It's probably negligible, though. Okay. I agree the second one is a better option. I have a couple of queries though: -The second BTA should be sent only after we get the Ack for the first one, i.e, we need to use bta_sync() for the first BTA, right? -We shouldn't send null packets and the 2 BTAs at all if we aren't using Automatic TE mode, is this correct? -Whose job should it be to send the null packet and the 2 BTAs, the dsi driver or the panel driver? Archit -- 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: [PATCH] OMAP: DSS2: DSI: Introduce sync_vc functions
On Wed, 2011-03-23 at 04:59 -0500, Taneja, Archit wrote: > From: Archit Taneja > > The DSI protocol engine has no interrupt for signalling the end of a Frame > transfer. The present approach is to send a BTA after DISPC generates a > FRAMEDONE interrupt, and unlock the dsi bus only when the BTA Ack is received. > > The assumption made with this approach was that OMAP will send a BTA only > after > the long packet corresponding to the last line is sent. However, it is > possible > that on the DISPC FRAMEDONE interrupt there are 2 (or more) lines of pixel > data > in the DSI line buffer. Hence, the BTA Ack could be received for the long > packet > corresponding to the second last line (or the third last and so on..). > Therefore, the current method doesn't ensure that the complete frame data is > sent before we start a new transfer. A similar explanation holds valid if we > send a BTA in between multiple short/long command packets from the slave port. > > Introduce dsi_sync_vc functions, based on Tomi Valkeinen's idea, which ensure > that the DSI Virtual Channel in use(update_channel) completes its previous > work > before proceeding to the next Frame/Command. > > For a frame update, the DSI driver now sends a callback to the Panel Driver > on the FRAMEDONE interrupt itself. The callback in the panel driver then > unlocks > the bus. dsi_sync_vc() functions are placed in dsi_vc_config_l4() and > dsi_vc_config_vp() to ensure that the previous task of the Virtual Channel is > completed. > > Signed-off-by: Archit Taneja > --- > Note: > Applies over the master branch of the tree: > http://gitorious.org/linux-omap-dss2/linux/commits/master > > drivers/video/omap2/dss/dsi.c | 180 ++-- > 1 files changed, 117 insertions(+), 63 deletions(-) Looks good, but this patch introduces one problem: handling the double BTA for DSI TE. Currently we aim DSI to be always in a state where one BTA has been sent. So when the next frame transfer starts, we can just send one BTA and we know we'll get two consecutive BTAs. This patch removes the BTA which is sent after the frame transfer, thus breaking the above system. This doesn't cause problems currently because panel-taal.c always sends columns and rows addresses before starting the frame update, and a BTA is sent after those configurations. But a simple optimization would be to only send column/row config if they have changed. And in that case we would not get a double BTA, and thus we'd never get the DSI TE. So how to solve that... Two ways come to my mind: - Track sent BTAs in dsi.c. Every time we send a packet, reset the counter. Every time we send a BTA, increase the counter. Thus at frame update we would know if we need to send an extra BTA. - Always reset the BTA "status" from the panel at the beginning of frame transfer. This could be done by sending a null packet. The second one is probably simpler and more failsafe as there's no state stored. The first one (as well as the current system) would go wrong if something strange happens, like the panel resets. However, the second one introduces some overhead, as we need to send a null packet and two BTAs (versus one BTA) for every frame. It's probably negligible, though. I think we should go with the second option for now. It's simpler and works (should, at least). We can study the first option later if we need to optimize this. 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
[PATCH] OMAP: DSS2: DSI: Introduce sync_vc functions
From: Archit Taneja
The DSI protocol engine has no interrupt for signalling the end of a Frame
transfer. The present approach is to send a BTA after DISPC generates a
FRAMEDONE interrupt, and unlock the dsi bus only when the BTA Ack is received.
The assumption made with this approach was that OMAP will send a BTA only after
the long packet corresponding to the last line is sent. However, it is possible
that on the DISPC FRAMEDONE interrupt there are 2 (or more) lines of pixel data
in the DSI line buffer. Hence, the BTA Ack could be received for the long packet
corresponding to the second last line (or the third last and so on..).
Therefore, the current method doesn't ensure that the complete frame data is
sent before we start a new transfer. A similar explanation holds valid if we
send a BTA in between multiple short/long command packets from the slave port.
Introduce dsi_sync_vc functions, based on Tomi Valkeinen's idea, which ensure
that the DSI Virtual Channel in use(update_channel) completes its previous work
before proceeding to the next Frame/Command.
For a frame update, the DSI driver now sends a callback to the Panel Driver
on the FRAMEDONE interrupt itself. The callback in the panel driver then unlocks
the bus. dsi_sync_vc() functions are placed in dsi_vc_config_l4() and
dsi_vc_config_vp() to ensure that the previous task of the Virtual Channel is
completed.
Signed-off-by: Archit Taneja
---
Note:
Applies over the master branch of the tree:
http://gitorious.org/linux-omap-dss2/linux/commits/master
drivers/video/omap2/dss/dsi.c | 180 ++--
1 files changed, 117 insertions(+), 63 deletions(-)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 5614164..cfbb772 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -2033,6 +2033,114 @@ static int dsi_force_tx_stop_mode_io(void)
return 0;
}
+static bool dsi_vc_is_enabled(int channel)
+{
+ return REG_GET(DSI_VC_CTRL(channel), 0, 0);
+}
+
+static void dsi_packet_sent_handler_vp(void *data, u32 mask)
+{
+ const int channel = dsi.update_channel;
+ u8 bit = dsi.te_enabled ? 30 : 31;
+
+ if (REG_GET(DSI_VC_TE(channel), bit, bit) == 0)
+ complete((struct completion *)data);
+}
+
+static int dsi_sync_vc_vp(int channel)
+{
+ int r = 0;
+ u8 bit;
+
+ DECLARE_COMPLETION_ONSTACK(completion);
+
+ bit = dsi.te_enabled ? 30 : 31;
+
+ r = dsi_register_isr_vc(channel, dsi_packet_sent_handler_vp,
+ &completion, DSI_VC_IRQ_PACKET_SENT);
+ if (r)
+ goto err0;
+
+ /* Wait for completion only if TE_EN/TE_START is still set */
+ if (REG_GET(DSI_VC_TE(channel), bit, bit)) {
+ if (wait_for_completion_timeout(&completion,
+ msecs_to_jiffies(10)) == 0) {
+ DSSERR("Failed to complete previous frame transfer\n");
+ r = -EIO;
+ goto err1;
+ }
+ }
+
+ dsi_unregister_isr_vc(channel, dsi_packet_sent_handler_vp,
+ &completion, DSI_VC_IRQ_PACKET_SENT);
+
+ return 0;
+err1:
+ dsi_unregister_isr_vc(channel, dsi_packet_sent_handler_vp, &completion,
+ DSI_VC_IRQ_PACKET_SENT);
+err0:
+ return r;
+}
+
+static void dsi_packet_sent_handler_l4(void *data, u32 mask)
+{
+ const int channel = dsi.update_channel;
+
+ if (REG_GET(DSI_VC_CTRL(channel), 5, 5) == 0)
+ complete((struct completion *)data);
+}
+
+static int dsi_sync_vc_l4(int channel)
+{
+ int r = 0;
+
+ DECLARE_COMPLETION_ONSTACK(completion);
+
+ r = dsi_register_isr_vc(channel, dsi_packet_sent_handler_l4,
+ &completion, DSI_VC_IRQ_PACKET_SENT);
+ if (r)
+ goto err0;
+
+ /* Wait for completion only if TX_FIFO_NOT_EMPTY is still set */
+ if (REG_GET(DSI_VC_CTRL(channel), 5, 5)) {
+ if (wait_for_completion_timeout(&completion,
+ msecs_to_jiffies(10)) == 0) {
+ DSSERR("Failed to complete previous l4 transfer\n");
+ r = -EIO;
+ goto err1;
+ }
+ }
+
+ dsi_unregister_isr_vc(channel, dsi_packet_sent_handler_l4,
+ &completion, DSI_VC_IRQ_PACKET_SENT);
+
+ return 0;
+err1:
+ dsi_unregister_isr_vc(channel, dsi_packet_sent_handler_l4,
+ &completion, DSI_VC_IRQ_PACKET_SENT);
+err0:
+ return r;
+}
+
+static int dsi_sync_vc(int channel)
+{
+ WARN_ON(!dsi_bus_is_locked());
+
+ WARN_ON(in_interrupt());
+
+ if (!dsi_vc_is_enabled(channel))
+ return 0;
+
+ switch (dsi.vc[channel].mode) {
+ case DSI_VC_MODE_VP:
+ return dsi_sync_vc_vp(channel);
+ case DSI_VC_MODE_L4:
+ return dsi_sync_vc_l4(channel);
+ defa
