Re: [PATCH v2] OMAP: DSS2: DSI: Introduce sync_vc functions

[Archit Taneja <archit@ti.com>]

On Tuesday 29 March 2011 12:42 PM, Valkeinen, Tomi wrote:
> On Tue, 2011-03-29 at 09:56 +0530, Archit Taneja wrote:
>> 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 functions, based on Tomi Valkeinen's idea, which ensure
>> that all the DSI Virtual Channels enabled complete their 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() functions are placed in dsi_vc_config_l4() and
>> dsi_vc_config_vp() to ensure that the previous tasks of the Virtual Channels are
>> completed.
>>
>> Signed-off-by: Archit Taneja<archit@ti.com>
>> ---
>> v2:
>> -Introduce dsi_sync() which syncs all VCs.
>> -Modify commit message for above change.
>
> We don't need to sync all VCs in dsi_vc_config_l4/vp(). There's no
> problem changing a VC between l4 and vp while other VCs are still
> transmitting.

Yes, makes sense.

>
> The problematic case where we need to sync all VCs is sending a BTA. BTA
> is not a VC-based thing, but a bus wide thing. If another VC was still
> transmitting data when we send a BTA, we cannot know when the BTA was
> actually sent, and to which packet the panel responds.

We can send a BTA per VC, and we get a BTA Ack corresponding to that VC 
(in DSI_VC_IRQSTATUS_i). I don't know what BTA per VC means, I guess 
OMAP DSI implementation deviates from the specification here.

We need to figure out why the bits exist per VC and not in something 
like DSI_CTRL and DSI_IRQSTATUS.

>
> But this BTA actually quite a messy thing overall. Consider a case where
> we use two VCs. We first set VC0 to send a bunch of packets. Then we use
> VC1 to send, say, READ_ID1, and after that we send a BTA to get the ID1.
>
> Now, even if send_bta() would call dsi_sync(), we don't know which
> packet was actually sent last. Was it the READ_ID1, or something from
> VC0?
>
> Luckily these things are not a problem in any current use case, as we
> have only one DSI device connected to a single DSI bus, and we can just
> use one VC.
>
> So... I think we could just use the v1 of this patch for now (if the
> dsi_sync change was the only change). This leaves us with the problem of
> sending the double BTA (which doesn't manifest currently), which can be
> fixed in a separate patch. And we need to think more about the solution
> for using multiple VCs reliably, and see to it later if there's need.
> How does that sound?

It sounds okay, i'll send a patch on which will implement sending double 
BTAs in the correct way.

Archit