On 07/28/2015 02:50 AM, Antti Palosaari wrote:
> On 07/27/2015 11:38 PM, Hans Verkuil wrote:
>> On 07/27/2015 10:21 PM, Antti Palosaari wrote:
>>> On 07/17/2015 05:43 PM, Hans Verkuil wrote:
>>>> On 07/16/2015 09:04 AM, Antti Palosaari wrote:
>>>>> HackRF SDR device has both receiver and transmitter. There is limitation
>>>>> that receiver and transmitter cannot be used at the same time
>>>>> (half-duplex operation). That patch implements transmitter support to
>>>>> existing receiver only driver.
>>>>>
>>>>> Cc: Hans Verkuil <hverkuil@xxxxxxxxx>
>>>>> Signed-off-by: Antti Palosaari <crope@xxxxxx>
>>>>> ---
>>>>> drivers/media/usb/hackrf/hackrf.c | 787 +++++++++++++++++++++++++++-----------
>>>>> 1 file changed, 572 insertions(+), 215 deletions(-)
>>>>>
>>>>
>>>>
>>>>> @@ -611,8 +751,15 @@ static int hackrf_queue_setup(struct vb2_queue *vq,
>>>>> unsigned int *nplanes, unsigned int sizes[], void *alloc_ctxs[])
>>>>> {
>>>>> struct hackrf_dev *dev = vb2_get_drv_priv(vq);
>>>>> + struct usb_interface *intf = dev->intf;
>>>>> + int ret;
>>>>>
>>>>> - dev_dbg(dev->dev, "nbuffers=%d\n", *nbuffers);
>>>>> + dev_dbg(&intf->dev, "nbuffers=%d\n", *nbuffers);
>>>>> +
>>>>> + if (test_and_set_bit(QUEUE_SETUP, &dev->flags)) {
>>>>> + ret = -EBUSY;
>>>>> + goto err;
>>>>> + }
>>>>
>>>> This doesn't work. The bit is only cleared when start_streaming fails or
>>>> stop_streaming is called. But the application can also call REQBUFS again
>>>> or just close the file handle, and then QUEUE_SETUP should also be cleared.
>>>>
>>>> But why is this here in the first place? It doesn't seem to do anything
>>>> useful (except mess up the v4l2-compliance tests).
>>>>
>>>> I've removed it and it now seems to work OK.
>>>
>>> It is there to block simultaneous use of receiver and transmitter.
>>> Device could operate only single mode at the time - receiving or
>>> transmitting. Driver shares streaming buffers.
>>>
>>> Any idea how I can easily implement correct blocking?
>>
>> Since each video_device struct has its own vb2_queue I wouldn't put the check
>> here. Instead, put the check in the start_streaming callback. And the check
>> is easy enough: if you want to start capturing, then call
>> vb2_is_streaming(&tx_vb2_queue). If you want to start output, then call
>> vb2_is_streaming(&rx_vb2_queue). If the other 'side' is streaming, then
>> return EBUSY.
>>
>> It's perfectly valid to allocate the buffers, but actually streaming is an
>> exclusive operation.
>
> Currently there is two queues, but only single buffer. If I do check on
> start_streaming() it is too late as buffers are queue during buf_queue()
> which is called earlier (and now both sides are added to same
> queued_bufs lists, which messes up).
It's not that you have a single buffer, it is that you have a single buffer list.
I'd say that you should make two buffer lists and use the appropriate one. They
really are independent, it's just that VIDIOC_STREAMON can run only one queue
at a time.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
