On 2018-01-30 06:07, Mauro Carvalho Chehab wrote:
> Em Thu,  4 Jan 2018 18:04:12 -0600
> Brad Love <b...@nextdimension.cc> escreveu:
>
>> Set appropriate bulk/ISOC transfer multiplier on capture start.
>> This sets ISOC transfer to 940 bytes (188 * 5)
>> This sets bulk transfer to 48128 bytes (188 * 256)
>>
>> The above values are maximum allowed according to Empia.
>>
>> Signed-off-by: Brad Love <b...@nextdimension.cc>
>> ---
>>  drivers/media/usb/em28xx/em28xx-core.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
>> b/drivers/media/usb/em28xx/em28xx-core.c
>> index ef38e56..67ed6a3 100644
>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
>>          dev->chip_id == CHIP_ID_EM28174 ||
>>          dev->chip_id == CHIP_ID_EM28178) {
>>              /* The Transport Stream Enable Register moved in em2874 */
>> +            if (dev->dvb_xfer_bulk) {
>> +                    /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 2 */
>> +                    em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> +                                    EM2874_R5D_TS1_PKT_SIZE :
>> +                                    EM2874_R5E_TS2_PKT_SIZE,
>> +                                    0xFF);
>> +            } else {
>> +                    /* TS2 Maximum Transfer Size = 188 * 5 */
>> +                    em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> +                                    EM2874_R5D_TS1_PKT_SIZE :
>> +                                    EM2874_R5E_TS2_PKT_SIZE, 0x05);
>> +            }
> Hmm... for ISOC, the USB descriptors inform the max transfer size, with
> are detected at probe time, on this part of em28xx_usb_probe:
>
>       if (size > dev->dvb_max_pkt_size_isoc) {
>               has_dvb = true; /* see NOTE (~) */
>               dev->dvb_ep_isoc = e->bEndpointAddress;
>               dev->dvb_max_pkt_size_isoc = size;
>               dev->dvb_alt_isoc = i;
>       }
>
> If we're touching TS PKT size register, it should somehow be
> aligned what's there. I mean, we should either do:
>
>                       em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>                                       EM2874_R5D_TS1_PKT_SIZE :
>                                       EM2874_R5E_TS2_PKT_SIZE, 
> dev->dvb_max_pkt_size_isoc / 188);
>
> Or the other way around, setting dev->dvb_max_pkt_size_isoc after
> writing to EM2874_R5D_TS1_PKT_SIZE or EM2874_R5E_TS2_PKT_SIZE.
>
> Not sure what's more accurate here: the USB descriptors or the
> contents of the TS size register. I doubt, I would stick with
> the USB descriptor info.
>
> Btw, I wander what happens if we write a bigger value than 5 to those
> registers. Would it support a bigger transfer size than 940 for ISOCH?
>
>
>
> Cheers,
> Mauro

Hi Mauro,

On the one ISOC device I have here, the usb endpoint
dvb_max_pkt_size_isoc is 940 during usb_probe and
EM2874_R5D_TS1_PKT_SIZE returns 5 when queried in start_streaming. I
just did a little checking and EM2874_R5D_TS1_PKT_SIZE accepted, and
returned the value I wrote all the way up to 32. The device is DVB
however, so I cannot test actual operation to see if it increases ISOC
packet size at all. We're just going on what Empia said here for the
maximum of 5.

I agree that this is probably more correct to use (dvb_max_pkt_size_isoc
/ 188) instead of a hardcoded 5. This at least would keep devices with
other multipliers happy. Your second method of querying the multiplier
before setting up the endpoint would require a re-organization of
usb_probe to move em28xx_init_dev higher.

I'll submit a v2 of this briefly.

Cheers,

Brad


Reply via email to