Re: [PATCHv2 10/56] drm/omap: dsi: drop virtual channel logic

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

On Wed, Apr 01, 2020 at 02:30:25PM +0300, Tomi Valkeinen wrote:
> On 25/02/2020 17:01, Laurent Pinchart wrote:
> > On Tue, Feb 25, 2020 at 12:20:40AM +0100, Sebastian Reichel wrote:
> >> This drops the virtual channel logic. Afterwards DSI clients
> >> request their channel number and get the virtual channel with
> >> the same number or -EBUSY if already in use.
> > 
> > I wonder why this level of indirection was used, allocating "virtual
> > VCs". A single virtual indirection should be enough :-) I may be missing
> > some context though, I'll defer that to Tomi, but for me,
> 
> I haven't reviewed the code yet, and it's been a long time since I wrote this code. But maybe this 
> explains at least some:
> 
> (I hope I remember this right)
> 
> DSI packets have virtual channel IDs (VCID). That's number 0-3 that needs to be in the packets.
> 
> DSI IP has virtual channel "blocks" (VC), with associated registers. So 4 VC register blocks. These 
> are not related to DSI virtual channel IDs in any way.
> 
> To do DSI transactions, you choose a VC, and program it. A VC can send data via video pipeline, or 
> transmit and receive data messages created with CPU. And in both cases, you need to include the VCID 
> in the transmissions, of course.
> 
> So, I think a normal use case could be a single panel, with VCID 0. To send video data and control 
> messages, you would use VC0 and VC1. VC0 would be configured for video data, and VC1 would be 
> configured for control messages.
> 
> And if I recall right, currently you first request a free VC from the IP with request_vc(). Then you 
> use set_vc_id(channel, id) to set the VCID, used when doing transactions with that VC.
> 
> So the virtual channel naming is pretty confusing in the DSI IP, in my opinion.

I wasn't aware of those details, thank you for the explanation. It's
quite confusing indeed, let's try to document the architecture in a
comment block at the beginning of the dsi.c file for later reference.

-- 
Regards,

Laurent Pinchart