<!-- Some styling for better description lists --><style type='text/css'>dt { font-weight: bold;float: left;display:inline;margin-right: 1em} dd { display:block; margin-left: 2em}</style>

   postmodern: hmm their proprietary installer seems to have installed some udev rules and .so's
   pinchartl: <u>hverkuil</u>: ping
   hverkuil: <u>pinchartl</u>: pong
   pinchartl: how are you doing ?
   <br> happy new year, slightly belated :-)
   <br> were you on holidays ?
   hverkuil: same to you!
   <br> Back from vacation. I didn't actually go anywhere (except for 10 days in the Netherlands), but I tried to stay away from computers as much as possible :-)
   pinchartl: that was a very wise decision
   <br> I will be away next week, but very likely with e-mail access unfortunately
   hverkuil: Just don't start your email reader :-)
   pinchartl: :-)
   <br> I suppose you have lots to catch up with
   <br> I wanted to briefly discuss the request API at some point
   <br> to ask you for guidance on a particular point
   hverkuil: Not too bad actually.
   <br> Is now a good time to discuss the request API?
   <br> Otherwise it'll be Friday.
   pinchartl: yes it is
   hverkuil: OK, let me get a coffee and then I'm ready.
   pinchartl: for reference, http://git.linuxtv.org/pinchartl/media.git/log/?h=drm/du/vsp1-kms/request
   <br> ok
   <br> ping me when you're back
   hverkuil: <u>pinchartl</u>: ping
   pinchartl: pong
   <br> the point I want to discuss with you is handling of links in the request API
   <br> I need to reconfigure the pipeline in potentially every request
   <br> (yes, lucky me)
   <br> from a userspace API I've currently added a request ID field to the media_link_desc structure
   hverkuil: I was hoping we could avoid that, but was afraid that this might happen.
   pinchartl: it might not be ideal
   <br> but it should do the job for now
   <br> my biggest concern is how to handle that in the kernel
   <br> first of all, have you given it any thought already ?
   hverkuil: A little bit.
   <br> I was thinking that this should be a lot easier with the new S_TOPOLOGY ioctl, where you could atomically pass in the new linkage.
   <br> I am not sure if MEDIA_IOC_SETUP_LINK works all that well with the request API.
   pinchartl: S_TOPOLOGY would be a much better fit
   hverkuil: On the other hand, the new ioctl isn't merged yet (and will apparently be postponed).
   pinchartl: but from a kernel point of view it won't change too much
   hverkuil: (someone needs to update me on the latest MC status)
   pinchartl: we'll get a list of links to be modified when the request is applied
   <br> whether they come from one S_TOPOLOGY call or multiple MEDIA_IOC_SETUP_LINK calls aggregated
   <br> we can also discuss the latest MC status, but one thing at a time please :-)
   hverkuil: Well, there is a difference: S_TOPOLOGY will overwrite the old links, whereas SETUP_LINK is incremental.
   pinchartl: neither will be applied immediately
   <br> I don't think it makes a big difference as far as I'm concerned
   <br> it's applying the links that bothers me right now
   hverkuil: what aspect of that bothers you? Locking, timing, userspace API, kernelspace API?
   <br> All of the above? :-)
   pinchartl: :-)
   <br> kernelspace
   <br> my driver currently performs the following
   <br> in the streamon handler it calls media_entity_pipeline_start()
   <br> the function will walk the graph
   <br> increase the streaming count of all connected entities
   <br> validate links
   <br> and store a pointer to the pipeline in the entities' pipe field
   <br> so far, so good
   <br> but if I later change links during streaming as a result of applying a request
   <br> all of this crumbles
   <br> as new entities will be added to the pipeline (not new in the sense of newly registered entities, just existing entities that were not part of the pipeline)
   <br> they won't have their pipe field set
   <br> their streaming count will be incorrect
   <br> it will also mess up the media_entity_pipeline_stop() call
   <br> as it will deal with a different pipeline than the one processed by media_entity_pipeline_start()
   <br> it also led me to realize that streamon, in the request case, is basically a noop from a device point of view
   <br> it updates various software states due to how our frameworks are architectured
   <br> but that's it
   <br> streamoff is nearly a noop too, the only operation that really matters there is the fact it releases all buffers. there's no other way to release buffers prepared or queued to the driver
   hverkuil: (phone call)
   pinchartl: no worries
   <br> ping me when you're back
   <br> when using the request API I actually don't need to call media_entity_pipeline_start() at streamon time
   <br> but
   <br> I have no way to know at that time whether the request API will be used or not
   <br> and I'm increasingly worried about races between requests and the normal API
   hverkuil: <u>pinchartl</u>: this call takes longer than expected, and I'll need to read up on the code as well.
   pinchartl: I'll be available the whole evening
   hverkuil: <u>pinchartl</u>: I don't understand what you mean with 'streamon is basically a noop': streamon calls the vb2 start_streaming op, which typically starts the DMA and does the pipeline validation, etc.
   pinchartl: except that it does none of that in request mode
   <br> pipeline is validated per-request as links can change
   <br> DMA is started when the request is applied
   hverkuil: Can you point me to that streamon code?
   pinchartl: isp_video.c
   <br> drivers/media/platform/isp_video.c
   <br> vsp1_video_start_streaming() and vsp1_video_streamon()
   <br> the condition vsp1_pipeline_ready() is never true in request mode in vsp1_video_streamon()
   <br> and vsp1_pipeline_setup() doesn't need to be called either
   <br> neither does vsp1_dl_list_get()
   <br> vsp1_video_start_streaming() turns into noop with requests
   hverkuil: that doesn't sound right. streamon should start streaming (provided enough buffers have been queued). So the first queued buffer applies the corresponding request. The streamon ioctl has nothing to do with requests, it just kicks off the DMA.
   pinchartl: requests are applied throught the request ioctl, not through queuuing buffers
   <br> s/throught/through/
   hverkuil: I'm really confused. The request ioctl can be used to apply a request now (MEDIA_REQ_CMD_APPLY), or it can queue the request (CMD_QUEUE), which implies queuing already prepared buffers for that request.
   <br> Neither case implies a streamon.
   pinchartl: I use CMD_QUEUE
   <br> so it queues a request with prepared buffers
   <br> and that has nothing to do with streamon, exactly
   <br> streamon does nothing
   <br> it's only at queue time that I have all the information I need to start DMA
   <br> remember that the device works in memory-to-memory mode
   <br> so DMA is one shot only
   <br> there's no such thing as starting the device
   <br> only kicking off a processing cycle with n input buffers and 1 output buffer
   hverkuil: Right, so when you say 'streamon is a noop' you really mean 'streamon is a noop for the vsp1'.
   pinchartl: yes
   <br> sorry :-)
   <br> not in general
   <br> I was talking about my case
   <br> and likely most of the request-based mem2mem drivers
   hverkuil: OK, that clarifies things a lot for me :-)
   <br> But in that case I would say that you rebuild/teardown the pipeline for every processing cycle. If I remember correctly, the vsp driver never needs to interface with external subdev drivers?
   <br> If so, then I suspect that it should be possible to do the pipeline validation when you call CMD_QUEUE.
   <br> Do you actually need to use the media pipeline functions? Those functions assume a mix of various subdevs, some SoC internal, some external to the SoC. But if everything is in the same driver, then I think you can simplify things substantially.
   <br> there is a lot of overhead in the pipeline functions so they are able to deal with worst-case scenarios, but that's not needed with the vsp1 as I understand it.
   <br> I have to go, but I'll check irc later tonight.
   pinchartl: correct, no external subdev. it's all internal
   <br> I plan to do pipeline validation when calling CMD_QUEUE, yes
   <br> regarding not calling media_entity_pipeline_start()
   <br> it might not be strictly needed indeed, I'll check that
   <br> one of the issues is that by streamon time we don't know whether we'll use requests or not
   <br> so the streamon handler can't skip pipeline validation and media_entity_pipeline_start() just for the request case
   <br> would you consider it acceptable for a driver to only support request mode ?
   hverkuil: <u>pinchartl</u>: too early to tell, to be honest.
   pinchartl: I'm worried of races between the request API and the "normal" API
   ***: prabhakarlad has left
   <br> awalls has left