On 01/30/2017 08:18 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 30 Jan 2017 17:15:36 -0200
> Mauro Carvalho Chehab <[email protected]> escreveu:
>
>> Em Mon, 9 Jan 2017 14:23:33 +0100
>> Hans Verkuil <[email protected]> escreveu:
>>
>>> See the v4 series for details:
>>>
>>> https://www.spinics.net/lists/linux-media/msg108737.html
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>> The following changes since commit 40eca140c404505c09773d1c6685d818cb55ab1a:
>>>
>>> [media] mn88473: add DVB-T2 PLP support (2016-12-27 14:00:15 -0200)
>>>
>>> are available in the git repository at:
>>>
>>> git://linuxtv.org/hverkuil/media_tree.git delta
>>>
>>> for you to fetch changes up to e6f199d01e7b8bc4436738b6c666fda31b9f3340:
>>>
>>> st-delta: debug: trace stream/frame information & summary (2017-01-09
>>> 14:16:45 +0100)
>>>
>>> ----------------------------------------------------------------
>>> Hugues Fruchet (10):
>>> Documentation: DT: add bindings for ST DELTA
>>> ARM: dts: STiH410: add DELTA dt node
>>> ARM: multi_v7_defconfig: enable STMicroelectronics DELTA Support
>>> MAINTAINERS: add st-delta driver
>>> st-delta: STiH4xx multi-format video decoder v4l2 driver
>>> st-delta: add memory allocator helper functions
>>> st-delta: rpmsg ipc support
>>> st-delta: EOS (End Of Stream) support
>>> st-delta: add mjpeg support
>>> st-delta: debug: trace stream/frame information & summary
>>
>> There is something wrong on this driver... even after applying all
>> patches, it complains that there's a for there that does nothing:
>>
>> drivers/media/platform/sti/delta/delta-v4l2.c:322 register_decoders() warn:
>> we never enter this loop
>> drivers/media/platform/sti/delta/delta-v4l2.c: In function
>> 'register_decoders':
>> drivers/media/platform/sti/delta/delta-v4l2.c:322:16: warning: comparison of
>> unsigned expression < 0 is always false [-Wtype-limits]
>> for (i = 0; i < ARRAY_SIZE(delta_decoders); i++) {
>> ^
Hi Mauro,
It's strange that you face this warning, code is like that:
/* registry of available decoders */
static const struct delta_dec *delta_decoders[] = {
#ifdef CONFIG_VIDEO_STI_DELTA_MJPEG
&mjpegdec,
#endif
};
and MJPEG config is enabled by default:
config VIDEO_STI_DELTA_MJPEG
bool "STMicroelectronics DELTA MJPEG support"
default y
so you should not encounter this warning.
On the other hand, you face issue on line 322 of delta-v4l2.c but in my
codebase, and also in Hans' git tree
(git://linuxtv.org/hverkuil/media_tree.git delta), this code is at line 323.
Anyway, in order to prevent such warning even if no decoder are selected
in config, I have reworked the code in v5 adding a "NULL"
element at the end of decoder array out of any config switch:
static const struct delta_dec *delta_decoders[] = {
#ifdef CONFIG_VIDEO_STI_DELTA_MJPEG
&mjpegdec,
#endif
NULL,
};
>>
>> On a first glance, it seems that the register_decoders() function is
>> reponsible to register the format decoders that the hardware
>> recognizes. If so, I suspect that this driver is deadly broken.
>>
>> Please be sure that the upstream driver works properly before
>> submitting it upstream.
>>
>> Also, please fix the comments to match the Kernel standard. E. g.
>> instead of:
>>
>> /* guard output frame count:
>> * - at least 1 frame needed for display
>> * - at worst 21
>> * ( max h264 dpb (16) +
>> * decoding peak smoothing (2) +
>> * user display pipeline (3) )
>> */
>>
>> It should be:
>>
>> /*
>> * guard output frame count:
>> * - at least 1 frame needed for display
>> * - at worst 21
>> * ( max h264 dpb (16) +
>> * decoding peak smoothing (2) +
>> * user display pipeline (3) )
>> */
>>
>> There are several similar occurrences among this patch series.
I apologize for this -unfortunately not raised by checkpatch, I will
have a look to fix it-
Multiple lines comments are now fixed in v5.
>
> Ah, forgot to comment, but it mentions a firmware. Does such firmware
> reside on some RAM memory? If so, how such firmware is loaded?
Firmware is loaded in coprocessor at system startup by remoteproc framework:
From "[GIT PULL] STi DT update for v4.11 round 1"
https://lkml.org/lkml/2017/1/12/525:
https://kernel.googlesource.com/pub/scm/linux/kernel/git/pchotard/sti/+/sti-dt-for-v4.11/arch/arm/boot/dts/stih407-family.dtsi
st231_delta: remote-processor {
compatible = "st,st231-rproc";
memory-region = <&delta_reserved>;
resets = <&softreset
STIH407_ST231_DMU_SOFTRESET>;
reset-names = "sw_reset";
clocks = <&clk_s_c0_flexgen CLK_ST231_DMU>;
clock-frequency = <600000000>;
st,syscfg = <&syscfg_core 0x224>;
#mbox-cells = <1>;
mbox-names = "vq0_rx", "vq0_tx", "vq1_rx", "vq1_tx";
mboxes = <&mailbox0 0 0>, <&mailbox3 0 1>, <&mailbox0 0
1>,
<&mailbox3 0 0>;
};
>
>>
>> Thanks,
>> Mauro
>>
>> Thanks,
>> Mauro
>
>
>
> Thanks,
> Mauro
>
Thanks for all,
Hugues.--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
