Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

[Krzysztof Kozlowski]

On 24.03.2016 09:26, Javier Martinez Canillas wrote:
> There are a bunch of media platform drivers under drivers/media/platform/
> that are for Samsung SoCs but are not being built with exynos_defconfig.
> 
> This patch enables them as a module to improve build coverage for these
> drivers and also to allow people use them with proper hardware if modules
> are installed. The S5P MFC driver wasn't enabled since it fails to probe.
> 
> Only the boolean Kconfig symbols are enabled as built-in, since drivers
> are not critical and also to keep the kernel binary image size as small
> as possible.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
> Changes in v2:
> - Remove S5P G2D and GSC symbols since overlaps with Exynos DRM functionality.
>   Suggested by Tobias Jakobi
> 
>  arch/arm/configs/exynos_defconfig | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Looks good.
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>


How about doing the same for multi_v7?


Best regards,

Krzysztof

Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

[Javier Martinez Canillas]

Hello Krzysztof,

On 03/23/2016 10:00 PM, Krzysztof Kozlowski wrote:
> On 24.03.2016 09:26, Javier Martinez Canillas wrote:
>> There are a bunch of media platform drivers under drivers/media/platform/
>> that are for Samsung SoCs but are not being built with exynos_defconfig.
>>
>> This patch enables them as a module to improve build coverage for these
>> drivers and also to allow people use them with proper hardware if modules
>> are installed. The S5P MFC driver wasn't enabled since it fails to probe.
>>
>> Only the boolean Kconfig symbols are enabled as built-in, since drivers
>> are not critical and also to keep the kernel binary image size as small
>> as possible.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> ---
>>
>> Changes in v2:
>> - Remove S5P G2D and GSC symbols since overlaps with Exynos DRM functionality.
>>   Suggested by Tobias Jakobi
>>
>>  arch/arm/configs/exynos_defconfig | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
> 
> Looks good.
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>

Thanks for the review.
 
> 
> How about doing the same for multi_v7?
>

I didn't consider multi_v7 because media drivers aren't necessary for booting
the boards and so it could increase build times for not real benefits in most
machines. But I can enable it in multi_v7 as a follow-up if you think that it
makes sense there too.
 
> 
> Best regards,
> 
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

[Krzysztof Kozlowski]

On 25.03.2016 12:15, Javier Martinez Canillas wrote:
>>
>> How about doing the same for multi_v7?
>>
> 
> I didn't consider multi_v7 because media drivers aren't necessary for booting
> the boards and so it could increase build times for not real benefits in most
> machines. But I can enable it in multi_v7 as a follow-up if you think that it
> makes sense there too.

I consider use cases of multi_v7 the same as exynos (except the
difference in policy: use always modules). This means that if it makes
sense to enable some media drivers on exynos (for build coverage, boot
testing and real usage) then it makes sense to do the same for multi_v7.

In ideal future we would move entirely from exynos-specific image to one
common image for testing: the multi_v7. It would make things simpler.

Best regards,
Krzysztof

Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

[Javier Martinez Canillas]

Hello Krzysztof,

On 03/28/2016 12:28 AM, Krzysztof Kozlowski wrote:
> On 25.03.2016 12:15, Javier Martinez Canillas wrote:
>>>
>>> How about doing the same for multi_v7?
>>>
>>
>> I didn't consider multi_v7 because media drivers aren't necessary for booting
>> the boards and so it could increase build times for not real benefits in most
>> machines. But I can enable it in multi_v7 as a follow-up if you think that it
>> makes sense there too.
> 
> I consider use cases of multi_v7 the same as exynos (except the
> difference in policy: use always modules). This means that if it makes
> sense to enable some media drivers on exynos (for build coverage, boot
> testing and real usage) then it makes sense to do the same for multi_v7.
>

Ok, thanks a lot for your suggestion. I'll post a patch for multi_v7 then.
 
> In ideal future we would move entirely from exynos-specific image to one
> common image for testing: the multi_v7. It would make things simpler.
> 
> Best regards,
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

[Javier Martinez Canillas]

Hello Krzysztof,

On 03/28/2016 01:14 AM, Javier Martinez Canillas wrote:
> 
> On 03/28/2016 12:28 AM, Krzysztof Kozlowski wrote:
>> On 25.03.2016 12:15, Javier Martinez Canillas wrote:
>>>>
>>>> How about doing the same for multi_v7?
>>>>
>>>
>>> I didn't consider multi_v7 because media drivers aren't necessary for booting
>>> the boards and so it could increase build times for not real benefits in most
>>> machines. But I can enable it in multi_v7 as a follow-up if you think that it
>>> makes sense there too.
>>
>> I consider use cases of multi_v7 the same as exynos (except the
>> difference in policy: use always modules). This means that if it makes
>> sense to enable some media drivers on exynos (for build coverage, boot
>> testing and real usage) then it makes sense to do the same for multi_v7.
>>
> 
> Ok, thanks a lot for your suggestion. I'll post a patch for multi_v7 then.
>  
>> In ideal future we would move entirely from exynos-specific image to one
>> common image for testing: the multi_v7. It would make things simpler.
>>
>> Best regards,
>> Krzysztof
>>
> 

Tobias mentioned on IRC that the exynos4-is driver conflicts with the
Exynos DRM driver since both exynos_drm_fimc and exynos-fimc-is use
the FIMC block.

So I think we should just discard $SUBJECT and instead add a Kconfig
check to not allow both drivers to be built as suggested by Tobias.

Like is the case for the other components (G2D, GSC, Mixer, etc).

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

[Krzysztof Kozlowski]

On 29.03.2016 00:15, Javier Martinez Canillas wrote:
> Tobias mentioned on IRC that the exynos4-is driver conflicts with the
> Exynos DRM driver since both exynos_drm_fimc and exynos-fimc-is use
> the FIMC block.
> 
> So I think we should just discard $SUBJECT and instead add a Kconfig
> check to not allow both drivers to be built as suggested by Tobias.
> 
> Like is the case for the other components (G2D, GSC, Mixer, etc).

Roger that.

BR,
Krzysztof

Re: [PATCH v2] ARM: exynos_defconfig: Enable Samsung media platform drivers as modules

[Javier Martinez Canillas]

[adding Seung-Woo Kim to cc]

Hello Krzysztof,

On 03/28/2016 09:25 PM, Krzysztof Kozlowski wrote:
> On 29.03.2016 00:15, Javier Martinez Canillas wrote:
>> Tobias mentioned on IRC that the exynos4-is driver conflicts with the
>> Exynos DRM driver since both exynos_drm_fimc and exynos-fimc-is use
>> the FIMC block.
>>
>> So I think we should just discard $SUBJECT and instead add a Kconfig
>> check to not allow both drivers to be built as suggested by Tobias.
>>

I posted the Kconfig change [0] but Seung-Woo explained [1] to me that
there are several instances of the FIMC and device nodes can use the
samsung,{isp,lcd}-wb properties to choose which instance to use.

>> Like is the case for the other components (G2D, GSC, Mixer, etc).
> 
> Roger that.
>

So enabling both the V4L2 and DRM drivers is a valid configuration and
$SUBJECT should be picked after all. Sorry for the confusion.

I'll re-post the patch and also include changes for multi_v7_defconfig
in the same series.

> BR,
> Krzysztof
> 

[0]: https://lkml.org/lkml/2016/3/28/494
[1]: https://lkml.org/lkml/2016/3/29/7

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America