Re: [PATCH v7 2/7] mtk-mdp: add driver to probe mdp components

[Yong Wu <yong.wu@mediatek.com>]

On Mon, 2021-09-06 at 00:23 +0800, houlong wei wrote:
> Hi Ezequiel,
> 
> Thank you for your attention to this series of patches. I answer
> partial of your questions below.
> Regards,
> Houlong
> 
> On Sat, 2021-09-04 at 20:34 +0800, Ezequiel Garcia wrote:
> > Hi Eizan,
> > 
> > Sorry for seeing this series so late.
> > 
> > On Wed, 25 Aug 2021 at 03:35, Eizan Miyamoto <eizan@chromium.org>
> > wrote:
> > > 
> > > Broadly, this patch (1) adds a driver for various MTK MDP
> > > components to
> > > go alongside the main MTK MDP driver, and (2) hooks them all
> > > together
> > > using the component framework.
> > > 
> > > (1) Up until now, the MTK MDP driver controls 8 devices in the
> > > device
> > > tree on its own. When running tests for the hardware video
> > > decoder,
> > > we
> > > found that the iommus and LARBs were not being properly
> > > configured.
> > 
> > Why were not being properly configured? What was the problem?
> > Why not fixing that instead?
> > 
> > Does this mean the driver is currently broken and unusable?
> 
> This series of patches are supplements to another series, please
> refer
> to  
> 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=515129c
> , which add device link between the mtk-iommu consumer and the mtk-
> larb 
> devices. Without that series of patches, the mtk-mdp driver can work
> well so far.
> But with that series, it seems the device link only can be
> established
> for the device which is registered as a platform driver. That's why
> Eizan adds this series of patches to make all mdp components to be
> registered as platform drivers.

The mt8173 mdp has several devices:
   mediatek,mt8173-mdp-rdma, mediatek,mt8173-mdp  
   mediatek,mt8173-mdp-rsz
   mediatek,mt8173-mdp-wdma
   mediatek,mt8173-mdp-wrot

Except the first one, the last three devices are not the standard
platform devices. Thus, they should not be the iommu consumer devices.

Question 1: The last three device don't work actually in mt8173 chrome,
right? or they access continuous buffers?

Question 2: The IOMMU device-link patchset just replaces the pm runtime
interfaces. It don't improve the mdp flow, also should not introduce
regression. thus, my v8 don't rebase this mdp patches. Does the iommu
patchset introduce regression for mdp?

@Eizan, @houlong, Could you help confirm this?
Thanks.

> 
> > 
> > > To
> > > configure them, a driver for each be added to mtk_mdp_comp so
> > > that
> > > mtk_iommu_add_device() can (eventually) be called from
> > > dma_configure()
> > > inside really_probe().
> > > 
> > > (2) The integration into the component framework allows us to
> > > defer
> > > the
> > > registration with the v4l2 subsystem until all the MDP-related
> > > devices
> > > have been probed, so that the relevant device node does not
> > > become
> > > available until initialization of all the components is complete.
> > > 
> > > Some notes about how the component framework has been integrated:
> > > 
> > > - The driver for the rdma0 component serves double duty as the
> > > "master"
> > >   (aggregate) driver as well as a component driver. This is a
> > > non-
> > > ideal
> > >   compromise until a better solution is developed. This device is
> > >   differentiated from the rest by checking for a "mediatek,vpu"
> > > property
> > >   in the device node.
> > > 
> > 
> > As I have stated in Yunfei, I am not convinced you need an async
> > framework
> > at all. It seems all these devices could have been linked together
> > in the device tree, and then have a master device to tie them.
> > 
> > I.e. something like
> > 
> > mdp {
> >   mdp_rdma0 {
> >   }
> >   mdp_rsz0 {
> >   }
> >   mdp_rsz1 {
> >   }
> > }
> > 
> 
> The commit message of the patch below explains that " If the mdp_*
> nodes are under an mdp sub-node, their corresponding platform device
> does not automatically get its iommu assigned properly."
> Please refer to 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/mediatek/mt8173.dtsi?h=v5.14.1&id=8127881f741dbbf9a1da9e9bc59133820160b217
> 
[snip]