mbox series

[v2,0/2] Refactor MDP driver and add dummy component driver

Message ID 20200506084039.249977-1-eizan@chromium.org (mailing list archive)
Headers show
Series Refactor MDP driver and add dummy component driver | expand

Message

Eizan Miyamoto May 6, 2020, 8:40 a.m. UTC
This series depends on all changes in the series:
https://patchwork.kernel.org/patch/11530275/

We are adding a dummy MDP component driver so that all the components
are properly configured with IOMMUs and LARBs. This is required for
us to get hardware video decode working in 4.19, and possibly newer
kernels.

Changes in v2:
- remove empty mtk_mdp_comp_init
- update documentation for enum mtk_mdp_comp_type
- remove comma after last element of mtk_mdp_comp_driver_dt_match

Eizan Miyamoto (2):
  [media] mtk-mdp: add driver to probe mdp components
  [media] mtk-mdp: use pm_runtime in MDP component driver

 drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 172 ++++++++++++++---
 drivers/media/platform/mtk-mdp/mtk_mdp_comp.h |  32 +--
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 182 ++++++++++++------
 drivers/media/platform/mtk-mdp/mtk_mdp_core.h |   1 +
 4 files changed, 286 insertions(+), 101 deletions(-)

Comments

Hans Verkuil June 24, 2020, 2:05 p.m. UTC | #1
On 06/05/2020 10:40, Eizan Miyamoto wrote:
> 
> This series depends on all changes in the series:
> https://patchwork.kernel.org/patch/11530275/

I plan on merging the v3 of this series.

> 
> We are adding a dummy MDP component driver so that all the components
> are properly configured with IOMMUs and LARBs. This is required for
> us to get hardware video decode working in 4.19, and possibly newer
> kernels.

What is the status of this series?

There was some discussion with Enric, but that didn't come to a conclusion,
I think.

Regards,

	Hans

> 
> Changes in v2:
> - remove empty mtk_mdp_comp_init
> - update documentation for enum mtk_mdp_comp_type
> - remove comma after last element of mtk_mdp_comp_driver_dt_match
> 
> Eizan Miyamoto (2):
>   [media] mtk-mdp: add driver to probe mdp components
>   [media] mtk-mdp: use pm_runtime in MDP component driver
> 
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 172 ++++++++++++++---
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.h |  32 +--
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 182 ++++++++++++------
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.h |   1 +
>  4 files changed, 286 insertions(+), 101 deletions(-)
>
Enric Balletbo i Serra June 26, 2020, 9:37 a.m. UTC | #2
Hi Eizan and Hans,

On 24/6/20 16:05, Hans Verkuil wrote:
> On 06/05/2020 10:40, Eizan Miyamoto wrote:
>>
>> This series depends on all changes in the series:
>> https://patchwork.kernel.org/patch/11530275/
> 
> I plan on merging the v3 of this series.
> 
>>
>> We are adding a dummy MDP component driver so that all the components
>> are properly configured with IOMMUs and LARBs. This is required for
>> us to get hardware video decode working in 4.19, and possibly newer
>> kernels.
> 
> What is the status of this series?
> 

First of all, sorry, I should have had answer before but for some reason I
missed the track of this patchset.

> There was some discussion with Enric, but that didn't come to a conclusion,
> I think.
> 

Yes, my main concern is how this driver is instantiated, it is using one of the
rdma nodes, see the rdma0 node vs rdma1, to instantiate the driver,

       mdp_rdma0: rdma@14001000 {
                compatible = "mediatek,mt8173-mdp-rdma";
                             "mediatek,mt8173-mdp";
                reg = <0 0x14001000 0 0x1000>;
                clocks = <&mmsys CLK_MM_MDP_RDMA0>,
                         <&mmsys CLK_MM_MUTEX_32K>;
                power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
                iommus = <&iommu M4U_PORT_MDP_RDMA0>;
                mediatek,larb = <&larb0>;
                mediatek,vpu = <&vpu>;
        };


        mdp_rdma1: rdma@14002000 {

                compatible = "mediatek,mt8173-mdp-rdma";
                reg = <0 0x14002000 0 0x1000>;
                clocks = <&mmsys CLK_MM_MDP_RDMA1>,
                         <&mmsys CLK_MM_MUTEX_32K>;
                power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
                iommus = <&iommu M4U_PORT_MDP_RDMA1>;
                mediatek,larb = <&larb4>;
        };


My point is that DT is to describe hardware not a trick to instantiate drivers,
so the "mediatek,mt8173-mdp" is completely unnecessary. We had the same issue
with the MMSYS driver and the mediatek DRM driver, and we solved by having the
mmsys driver instantiating the drm driver. I think we should apply the same
solution here, see [1] for reference. If you want to continue the discussion
maybe would be better


Thanks,
 Enric

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/soc/mediatek/mtk-mmsys.c?id=667c769246b01c53ad0925d603d2a2531abd3ef2

> Regards,
> 
> 	Hans
> 
>>
>> Changes in v2:
>> - remove empty mtk_mdp_comp_init
>> - update documentation for enum mtk_mdp_comp_type
>> - remove comma after last element of mtk_mdp_comp_driver_dt_match
>>
>> Eizan Miyamoto (2):
>>   [media] mtk-mdp: add driver to probe mdp components
>>   [media] mtk-mdp: use pm_runtime in MDP component driver
>>
>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 172 ++++++++++++++---
>>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.h |  32 +--
>>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 182 ++++++++++++------
>>  drivers/media/platform/mtk-mdp/mtk_mdp_core.h |   1 +
>>  4 files changed, 286 insertions(+), 101 deletions(-)
>>
>