Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick)

Mauro Carvalho Chehab
2011-12-25T18:13:38Z

On 25-12-2011 12:04, Dennis Sperlich wrote:
> On 25.12.2011 11:52, Mauro Carvalho Chehab wrote:
>> On 24-12-2011 19:58, Dennis Sperlich wrote:
>>> Hi,
>>>
>>> I have a Terratec Cinergy HTC Stick an tried the new support for the DVB-C part. It works for SD material (at least for free receivable stations, I tried afair only QAM64), but did not for HD stations (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD (via KabelDeutschland). The HD material was just digital artefacts, as far as mplayer could decode it. When I did a dumpstream and looked at the resulting file size I got something about 1MB/s which seems a little too low, because SD was already about 870kB/s. After looking around I found a solution in increasing the isoc_dvb_max_packetsize from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s and looked good. I'm not sure, whether this is the correct fix, but it works for me.
>>>
>>> If you need more testing pleas tell.
>>>
>>> Regards,
>>> Dennis
>>>
>>>
>>>
>>> index 804a4ab..c518d13 100644
>>> --- a/drivers/media/video/em28xx/em28xx-core.c
>>> +++ b/drivers/media/video/em28xx/em28xx-core.c
>>> @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev)
>>> * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T
>>> * but not enough for 44 Mbit DVB-C.
>>> */
>>> - packet_size = 752;
>>> + packet_size = 940;
>>> }
>>>
>>> return packet_size;
>> As you can see there at the code, the packet size depends on the chipset, as
>> not all will support 940 for packet size.
>>
>> Could you please provide us what was the chip detected id?
>>
>> It should be a message on your dmesg like:
>>
>> chip ID is em2870
>> or
>> em28xx chip ID = 38
>>
>> The patch should change it only for your specific chipset model, in order to
>> avoid regressions to other supported chipsets, like:
>>
>> int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev)
>> {
>> unsigned int chip_cfg2;
>> unsigned int packet_size;
>>
>> switch (dev->chip_id) {
>> case CHIP_ID_EM2710:
>> case CHIP_ID_EM2750:
>> case CHIP_ID_EM2800:
>> case CHIP_ID_EM2820:
>> ...
>> case CHIP_ID_foo:
>> packet_size = 940;
>> ...
>> }
>>
>> return packet_size;
>> }
>>
>> The case you're touching seems to be for em2884, but the switch covers both
>> em2884 and em28174 (plus the default for newer chipsets).
>
> dmesg says: em28xx #0: chip ID is em2884
>
> so a patch would be more like:
>
> index 804a4ab..9280251 100644
> --- a/drivers/media/video/em28xx/em28xx-core.c
> +++ b/drivers/media/video/em28xx/em28xx-core.c
> @@ -1151,6 +1151,8 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev)
> packet_size = 564;
> break;
> case CHIP_ID_EM2884:
> + packet_size = 940;
> + break;
> case CHIP_ID_EM28174:
> default:
> /*
>
>> Maybe Michael/Devin may have something to say, with regards to a way to detect
>> the maximum supported packet size by each specific em28xx model.
>
> This would be fine, I tried also 1128 (6 times 188), but then mplayer did not play any more.
> I then tried 1034, but then I got the message " submit of urb 0 failed (error=-90)", so I guess
> there have to be integer multiples of 188.

The maximum packet size is described at the USB descriptors.

The code at em28xx_set_alternate() seeks for the alternates and selects
the one that will get the best alternate for the analog mode.

There, you'll see a code that gets the best alternate for analog,
and selects the proper packet size for it.

For DVB, it should be a little different, as, on DVB, the driver doesn't
know, in advance, what's the streaming rate. So, the driver needs to get
a high enough packet size to fit for the bandwidth needs.

Also, for DVB, the driver does:
usb_set_interface(dev->udev, 0, 1);

(e. g. it always use alternate 1)

So, in thesis, that function could be as simple as:
dev->alt_max_pkt_size[1];

(if the max packet size for alternate 1 would work for all chips)

Or, eventually, the code might be using other alternates for HD,
but I'm not sure if those chipsets support a different alternate for DVB.

Regards,
Mauro