On 08/10/2020 16:02, Alexandre Courbot wrote: > Hi Hans, thanks for taking the time to look at this! > > On Thu, Oct 8, 2020 at 10:12 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote: >> >> On 08/10/2020 15:07, Hans Verkuil wrote: >>> Hi Alexandre, >>> >>> On 04/10/2020 14:22, Alexandre Courbot wrote: >>>> The addition of MT8183 support added a dependency on the SCP remoteproc >>>> module. However the initial patch used the "select" Kconfig directive, >>>> which may result in the SCP module to not be compiled if remoteproc was >>>> disabled. In such a case, mtk-vcodec would try to link against >>>> non-existent SCP symbols. "select" was clearly misused here as explained >>>> in kconfig-language.txt. >>>> >>>> Replace this by a "depends" directive on at least one of the VPU and >>>> SCP modules, to allow the driver to be compiled as long as one of these >>>> is enabled, and adapt the code to support this new scenario. >>>> >>>> Also adapt the Kconfig text to explain the extra requirements for MT8173 >>>> and MT8183. >>>> >>>> Reported-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> >>>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxxxx> >>>> Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> # build-tested >>>> --- >>>> drivers/media/platform/Kconfig | 10 +-- >>>> .../media/platform/mtk-vcodec/mtk_vcodec_fw.c | 72 ++++++++++++------- >>>> 2 files changed, 54 insertions(+), 28 deletions(-) >>>> >>>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig >>>> index a3cb104956d5..98eb62e49ec2 100644 >>>> --- a/drivers/media/platform/Kconfig >>>> +++ b/drivers/media/platform/Kconfig >>>> @@ -253,14 +253,16 @@ config VIDEO_MEDIATEK_VCODEC >>>> depends on MTK_IOMMU || COMPILE_TEST >>>> depends on VIDEO_DEV && VIDEO_V4L2 >>>> depends on ARCH_MEDIATEK || COMPILE_TEST >>>> + depends on VIDEO_MEDIATEK_VPU || MTK_SCP >>> >>> Close, but no cigar. >>> >>> If VIDEO_MEDIATEK_VPU=y and MTK_SCP=m, then VIDEO_MEDIATEK_VCODEC can be configured >>> to y, and then it won't be able to find the scp_ functions. >>> >>> To be honest, I'm not sure how to solve this. >> >> Found it. Add this: >> >> depends on MTK_SCP || !MTK_SCP >> depends on VIDEO_MEDIATEK_VPU || !VIDEO_MEDIATEK_VPU >> >> Ugly as hell, but it appears to be the correct incantation for this. > > But doesn't it mean that the driver can be compiled if !MTK_SCP and > !VIDEO_MEDIATEK_VPU? That's the one case we want to avoid. No, because you still have: depends on VIDEO_MEDIATEK_VPU || MTK_SCP So at least one of these must be set. Just try it :-) Regards, Hans
