[PATCH 0/3] drm/exynos: Kconfig dependency fixes

[PATCH 0/3] drm/exynos: Kconfig dependency fixes

[Javier Martinez Canillas]

Hello Inki,

This patch series contains some fixes for the Kconfig symbol dependencies
of the Exynos DRM driver. They make sure that the Exynos DRM components
and the media platform drivers that makes use of the same HW IP block are
not enabled at the same time.

Best regards,
Javier


Javier Martinez Canillas (3):
  drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
  drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
  drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n

 drivers/gpu/drm/exynos/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.5.0

[PATCH 1/3] drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency

[Javier Martinez Canillas]

Commit 254d4d111ee1 ("drm/exynos: Add dependency for G2D in Kconfig") made
the DRM_EXYNOS_G2D symbol to only be selectable if the s5p-g2d V4L2 driver
is not enabled, since both use the same HW IP block.

But added the dependency as depends on !VIDEO_SAMSUNG_S5P_G2D which isn't
correct since Kconfig expressions are not boolean but tristate. So it will
only evaluate to 'n' if VIDEO_SAMSUNG_S5P_G2D=y but it will evaluate to m
if VIDEO_SAMSUNG_S5P_G2D=m.

This means that both the V4L2 and DRM drivers can be enabled if the former
is enabled as a module, which is not what we want.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/gpu/drm/exynos/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index f17d39279596..baddf33fb475 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -94,7 +94,7 @@ comment "Sub-drivers"
 
 config DRM_EXYNOS_G2D
 	bool "G2D"
-	depends on !VIDEO_SAMSUNG_S5P_G2D
+	depends on VIDEO_SAMSUNG_S5P_G2D=n
 	select FRAME_VECTOR
 	help
 	  Choose this option if you want to use Exynos G2D for DRM.
-- 
2.5.0

[PATCH 2/3] drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency

[Javier Martinez Canillas]

Commit aeefb36832e5 ("drm/exynos: gsc: add device tree support and remove
usage of static mappings") made the DRM_EXYNOS_GSC Kconfig symbol to only
be selectable if the exynos-gsc V4L2 driver isn't enabled, since both use
the same HW IP block.

But added the dependency as depends on !VIDEO_SAMSUNG_EXYNOS_GSC which is
not correct since Kconfig expressions are not boolean but tristate. So it
will only evaluate to 'n' if VIDEO_SAMSUNG_EXYNOS_GSC=y but will evaluate
to 'm' if VIDEO_SAMSUNG_S5P_G2D=m.

This means that both the V4L2 and DRM drivers can be enabled if the former
is enabled as a module, which is not what we want.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
---

 drivers/gpu/drm/exynos/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index baddf33fb475..8c27037787fc 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -118,7 +118,7 @@ config DRM_EXYNOS_ROTATOR
 
 config DRM_EXYNOS_GSC
 	bool "GScaler"
-	depends on DRM_EXYNOS_IPP && ARCH_EXYNOS5 && !VIDEO_SAMSUNG_EXYNOS_GSC
+	depends on DRM_EXYNOS_IPP && ARCH_EXYNOS5 && VIDEO_SAMSUNG_EXYNOS_GSC=n
 	help
 	  Choose this option if you want to use Exynos GSC for DRM.
 
-- 
2.5.0

[PATCH 2/3] drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency

[Javier Martinez Canillas]

Hello Inki,

On 03/28/2016 09:28 PM, Javier Martinez Canillas wrote:
> Commit aeefb36832e5 ("drm/exynos: gsc: add device tree support and remove
> usage of static mappings") made the DRM_EXYNOS_GSC Kconfig symbol to only
> be selectable if the exynos-gsc V4L2 driver isn't enabled, since both use
> the same HW IP block.
> 
> But added the dependency as depends on !VIDEO_SAMSUNG_EXYNOS_GSC which is
> not correct since Kconfig expressions are not boolean but tristate. So it
> will only evaluate to 'n' if VIDEO_SAMSUNG_EXYNOS_GSC=y but will evaluate
> to 'm' if VIDEO_SAMSUNG_S5P_G2D=m.
> 
> This means that both the V4L2 and DRM drivers can be enabled if the former
> is enabled as a module, which is not what we want.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---

I see that only patch 1/3 from this series was picked but according to the
conversation with Seung-Woo [0] in the cover letter, the GSC v4l2 and drm
drivers don't support to be simultaneous enabled like is the case for FIMC.

So patch 3/3 is the only one of the series that should be dropped and this
one picked as well.

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

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

[PATCH 3/3] drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n

[Javier Martinez Canillas]

Exynos DRM driver components are only enabled if other drivers that make
use of the same HW IP block are not enabled. That's the case for the G2D,
GSC, Mixer and HDMI drivers.

The FIMC is also shared by the DRM and a V4L2 driver, so the same has to
be added as a dependency for the Exynos FIMC DRM driver.

Suggested-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 drivers/gpu/drm/exynos/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 8c27037787fc..f9b92c3825ea 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -106,7 +106,7 @@ config DRM_EXYNOS_IPP
 
 config DRM_EXYNOS_FIMC
 	bool "FIMC"
-	depends on DRM_EXYNOS_IPP && MFD_SYSCON
+	depends on DRM_EXYNOS_IPP && MFD_SYSCON && VIDEO_S5P_FIMC=n
 	help
 	  Choose this option if you want to use Exynos FIMC for DRM.
 
-- 
2.5.0

[PATCH 0/3] drm/exynos: Kconfig dependency fixes

[Inki Dae]

Hi Javier,

Thanks for your patch set.

Will merge them if there is no issue.

Thanks,
Inki Dae

2016? 03? 29? 10:28? Javier Martinez Canillas ?(?) ? ?:
> Hello Inki,
> 
> This patch series contains some fixes for the Kconfig symbol dependencies
> of the Exynos DRM driver. They make sure that the Exynos DRM components
> and the media platform drivers that makes use of the same HW IP block are
> not enabled at the same time.
> 
> Best regards,
> Javier
> 
> 
> Javier Martinez Canillas (3):
>   drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
>   drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
>   drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n
> 
>  drivers/gpu/drm/exynos/Kconfig | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

[PATCH 0/3] drm/exynos: Kconfig dependency fixes

[Seung-Woo Kim]

Hi Javier,

On 2016? 03? 29? 10:28, Javier Martinez Canillas wrote:
> Hello Inki,
> 
> This patch series contains some fixes for the Kconfig symbol dependencies
> of the Exynos DRM driver. They make sure that the Exynos DRM components
> and the media platform drivers that makes use of the same HW IP block are
> not enabled at the same time.
> 
> Best regards,
> Javier
> 
> 
> Javier Martinez Canillas (3):
>   drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
>   drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
>   drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n

In G2D case, there is only one instance, but for the other cases, there
are several instances and in my environment, I enable both drivers on
v4l2 and drm FIMC/GSC.

So, IMHO, the not-enabled v4l2 dependency is not really required for drm
fimc and drm gsc.

Best Regards,
- Seung-Woo Kim

> 
>  drivers/gpu/drm/exynos/Kconfig | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

[PATCH 0/3] drm/exynos: Kconfig dependency fixes

[Javier Martinez Canillas]

Hello Seung-Woo,

Thanks a lot for your feedback.

On 03/28/2016 09:46 PM, Seung-Woo Kim wrote:
> Hi Javier,
> 
> On 2016? 03? 29? 10:28, Javier Martinez Canillas wrote:
>> Hello Inki,
>>
>> This patch series contains some fixes for the Kconfig symbol dependencies
>> of the Exynos DRM driver. They make sure that the Exynos DRM components
>> and the media platform drivers that makes use of the same HW IP block are
>> not enabled at the same time.
>>
>> Best regards,
>> Javier
>>
>>
>> Javier Martinez Canillas (3):
>>   drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
>>   drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
>>   drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n
> 
> In G2D case, there is only one instance, but for the other cases, there
> are several instances and in my environment, I enable both drivers on
> v4l2 and drm FIMC/GSC.
> 
> So, IMHO, the not-enabled v4l2 dependency is not really required for drm
> fimc and drm gsc.
>

I'm confused, it was you who added the depends on !VIDEO_SAMSUNG_EXYNOS_GSC
for DRM_EXYNOS_GSC in commit aeefb36832e5 ("drm/exynos: gsc: add device tree
support and remove usage of static mappings").

>From the commit message "The driver cannot be used simultaneously with V4L2
Mem2Mem GScaller driver thought". Did that assumption changed and the depend
should be removed then? or maybe I misunderstood what you meant.

Now, I'm not really sure about FIMC either, it was feedback I got from this
patch [0]. Could you please take a look to that and let me know if enabling
these drivers simultaneously makes sense then?

> Best Regards,
> - Seung-Woo Kim
> 
>>
>>  drivers/gpu/drm/exynos/Kconfig | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
> 

[0]: https://lkml.org/lkml/2016/3/23/292

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

[PATCH 0/3] drm/exynos: Kconfig dependency fixes

[Seung-Woo Kim]

Hello Javier,

On 2016? 03? 29? 11:41, Javier Martinez Canillas wrote:
> Hello Seung-Woo,
> 
> Thanks a lot for your feedback.
> 
> On 03/28/2016 09:46 PM, Seung-Woo Kim wrote:
>> Hi Javier,
>>
>> On 2016? 03? 29? 10:28, Javier Martinez Canillas wrote:
>>> Hello Inki,
>>>
>>> This patch series contains some fixes for the Kconfig symbol dependencies
>>> of the Exynos DRM driver. They make sure that the Exynos DRM components
>>> and the media platform drivers that makes use of the same HW IP block are
>>> not enabled at the same time.
>>>
>>> Best regards,
>>> Javier
>>>
>>>
>>> Javier Martinez Canillas (3):
>>>   drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
>>>   drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
>>>   drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n
>>
>> In G2D case, there is only one instance, but for the other cases, there
>> are several instances and in my environment, I enable both drivers on
>> v4l2 and drm FIMC/GSC.
>>
>> So, IMHO, the not-enabled v4l2 dependency is not really required for drm
>> fimc and drm gsc.
>>
> 
> I'm confused, it was you who added the depends on !VIDEO_SAMSUNG_EXYNOS_GSC
> for DRM_EXYNOS_GSC in commit aeefb36832e5 ("drm/exynos: gsc: add device tree
> support and remove usage of static mappings").

Yes, you are right. Originally, my goal on the GSC was bringing optional
flag for setting isp mode or wb mode like FIMC in tizen.org git tree.

https://review.tizen.org/git/?p=platform/kernel/linux-exynos.git;a=blobdiff;f=Documentation/devicetree/bindings/media/exynos5-gsc.txt;h=d526777a3abd04d244c46fdd729e3df93bb86917;hp=0604d42f38d1941526d47ad11a958a2a83797f97;hb=751cd6d88d9620c83042641b52fdd244408a3947;hpb=7c7ab44f86d64ba6a6733da8f201a26a6c0e807f

But only devicetree part of GSC was upstreamed and simultaneous enabling
both v4l2 gsc and drm gsc driver is removed. The only reason I set both
driver simultaneously was enabling video codec, exynos-mfc with gsc to
convert RGB plane.

I know Marek already has plan to integrate yuv plane feature of GSC to
DRM KMS. Also, for GSC, simultaneous setup is alredy remove, so your
patch seems better.

> 
>>From the commit message "The driver cannot be used simultaneously with V4L2
> Mem2Mem GScaller driver thought". Did that assumption changed and the depend
> should be removed then? or maybe I misunderstood what you meant.
> 
> Now, I'm not really sure about FIMC either, it was feedback I got from this
> patch [0]. Could you please take a look to that and let me know if enabling
> these drivers simultaneously makes sense then?

About FIMC, there is still simultaneous setup for both v4l2 and drm
driver with devicetree binding flags, samsung,isp-wb and samsung,lcd-wb.

If on the FIMC instance of dt node, samsung,lcd-wb is set, then drm
driver is probed, otherwise v4l2 driver is probed.

Simultaneous FIMC driver is also only for RGB converting of video codec
planes like GSC at least to me.

Marek, do you have any idea about the simultaneous setup for fimc and gsc?

Best Regards,
- Seung-Woo Kim

> 
>> Best Regards,
>> - Seung-Woo Kim
>>
>>>
>>>  drivers/gpu/drm/exynos/Kconfig | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>
> 
> [0]: https://lkml.org/lkml/2016/3/23/292
> 
> Best regards,
> 

-- 
Seung-Woo Kim
Samsung Software R&D Center
--

[PATCH 0/3] drm/exynos: Kconfig dependency fixes

[Javier Martinez Canillas]

Hello Seung-Woo,

On 03/29/2016 12:25 AM, Seung-Woo Kim wrote:
> Hello Javier,
> 
> On 2016? 03? 29? 11:41, Javier Martinez Canillas wrote:
>> Hello Seung-Woo,
>>
>> Thanks a lot for your feedback.
>>
>> On 03/28/2016 09:46 PM, Seung-Woo Kim wrote:
>>> Hi Javier,
>>>
>>> On 2016? 03? 29? 10:28, Javier Martinez Canillas wrote:
>>>> Hello Inki,
>>>>
>>>> This patch series contains some fixes for the Kconfig symbol dependencies
>>>> of the Exynos DRM driver. They make sure that the Exynos DRM components
>>>> and the media platform drivers that makes use of the same HW IP block are
>>>> not enabled at the same time.
>>>>
>>>> Best regards,
>>>> Javier
>>>>
>>>>
>>>> Javier Martinez Canillas (3):
>>>>   drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
>>>>   drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
>>>>   drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n
>>>
>>> In G2D case, there is only one instance, but for the other cases, there
>>> are several instances and in my environment, I enable both drivers on
>>> v4l2 and drm FIMC/GSC.
>>>
>>> So, IMHO, the not-enabled v4l2 dependency is not really required for drm
>>> fimc and drm gsc.
>>>
>>
>> I'm confused, it was you who added the depends on !VIDEO_SAMSUNG_EXYNOS_GSC
>> for DRM_EXYNOS_GSC in commit aeefb36832e5 ("drm/exynos: gsc: add device tree
>> support and remove usage of static mappings").
> 
> Yes, you are right. Originally, my goal on the GSC was bringing optional
> flag for setting isp mode or wb mode like FIMC in tizen.org git tree.
> 
> https://review.tizen.org/git/?p=platform/kernel/linux-exynos.git;a=blobdiff;f=Documentation/devicetree/bindings/media/exynos5-gsc.txt;h=d526777a3abd04d244c46fdd729e3df93bb86917;hp=0604d42f38d1941526d47ad11a958a2a83797f97;hb=751cd6d88d9620c83042641b52fdd244408a3947;hpb=7c7ab44f86d64ba6a6733da8f201a26a6c0e807f
> 
> But only devicetree part of GSC was upstreamed and simultaneous enabling
> both v4l2 gsc and drm gsc driver is removed. The only reason I set both
> driver simultaneously was enabling video codec, exynos-mfc with gsc to
> convert RGB plane.
> 
> I know Marek already has plan to integrate yuv plane feature of GSC to
> DRM KMS. Also, for GSC, simultaneous setup is alredy remove, so your
> patch seems better.
>

Ok, thanks for the confirmation. So at least patch 1/3 and 3/3 are needed then.
 
>>
>> >From the commit message "The driver cannot be used simultaneously with V4L2
>> Mem2Mem GScaller driver thought". Did that assumption changed and the depend
>> should be removed then? or maybe I misunderstood what you meant.
>>
>> Now, I'm not really sure about FIMC either, it was feedback I got from this
>> patch [0]. Could you please take a look to that and let me know if enabling
>> these drivers simultaneously makes sense then?
> 
> About FIMC, there is still simultaneous setup for both v4l2 and drm
> driver with devicetree binding flags, samsung,isp-wb and samsung,lcd-wb.
> 
> If on the FIMC instance of dt node, samsung,lcd-wb is set, then drm
> driver is probed, otherwise v4l2 driver is probed.
> 

Interesting, I didn't know about these DT properties. I see that some FIMC nodes
are using both though like in arch/arm/boot/dts/exynos4x12.dtsi. Is that a bug?

So if both the DRM and V4L2 drivers can be enabled at the same time for FIMC,
then patch 3/3 should be dropped and instead the exynos_defconfig patch that
enables CONFIG_VIDEO_S5P_FIMC should be picked IMHO.

> Simultaneous FIMC driver is also only for RGB converting of video codec
> planes like GSC at least to me.
> 
> Marek, do you have any idea about the simultaneous setup for fimc and gsc?
> 
> Best Regards,
> - Seung-Woo Kim
> 
>>
>>> Best Regards,
>>> - Seung-Woo Kim
>>>
>>>>
>>>>  drivers/gpu/drm/exynos/Kconfig | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>
>>
>> [0]: https://lkml.org/lkml/2016/3/23/292
>>
>> Best regards,
>>
> 

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